Skip to content

fix(plan-mode): render plan_ready output as Markdown - #293

Open
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render
Open

fix(plan-mode): render plan_ready output as Markdown#293
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render

Conversation

@Matt-qwq

@Matt-qwqMatt-qwq commented Aug 29, 2026

Copy link
Copy Markdown

Problem

plan_ready renders the finalized plan as raw Markdown source in the TUI.

extensions/plan-mode/index.ts registers plan_ready with only an execute handler and no renderResult, so the TUI falls back to its default plain-text renderer: headings, lists, and code fences appear as literal #, -, and ```
characters instead of rendered text.

Affects every Plan Mode user — the plan is the one artifact Plan Mode exists to produce, and it is meant to be read, not inspected as source.

No existing issue tracks this. I searched plan_ready, renderResult, and Markdown-rendering across open and closed issues in openpi-dev/openpi; the closest hits (#28, #27, #18, #105, #101, #96, #93, #67, #40) are all about other
subsystems.

Value

Every other extension tool that returns human-readable prose already supplies a renderer — there are 19 renderResult implementations across goal, tasks, git-read, file-search, workflows, subagents, ask-user,
background-terminals, and file-mutation-display. plan_ready is the outlier. Rendering it makes Plan Mode's output consistent with the rest of OpenPI and lets the user read the plan without decoding Markdown syntax.

Approach

Add renderResult to the plan_ready registration, rendering result.details.plan with the Markdown component from @earendil-works/pi-tui using getMarkdownTheme() — the same treatment renderWaitResult() gives subagent results
(extensions/subagents/src/ui/wait-result.ts:59).

Two deliberate choices:

  • Collapsed state shows a bounded preview. Pi passes the expanded flag to custom renderers but never truncates their output — the renderer owns the collapsed/expanded contract (same pattern as git-read and renderWaitResult). Collapsed renders Plan ready · N lines · <to expand> plus the first 10 plan lines and a ... (N more lines) tail; the hint and tail disappear when the plan fits the preview. Only the expanded state renders the full Markdown. Without this, a plan near the 48 KiB cap floods the transcript and Ctrl+O produces no visible change.
  • No re-sanitizing.execute already stores sanitizeTerminalText(params.plan), so details.plan is clean on the way in.

Validation

Ran on WSL2 (Fedora 44), Node v22.23.1, bun 1.3.14:

bun run check # biome format (257 files), biome lint --error-on-warnings, tsc --noEmit
bun run test # node:test 968 cases: 967 pass / 0 fail / 1 skipped
# vitest 1 file: 30 tests passed

bun run test grows from 959 to 968 cases — the 9 in tests/extensions/plan-mode/result-rendering.test.ts. Those were verified red against the pre-fix code: with renderResult removed, all nine fail with plan_ready must supply renderResult, so they catch the regression rather than document it.

Manual TUI check. Loaded the checkout with pi install, ran /plan, and called plan_ready with a plan exercising headings, ordered and nested lists, a block quote, a table, inline code, bold, italics, links, three fenced code shapes,
and a thematic break. In the TUI: heading markers are consumed, the table renders with box-drawing characters, the quote gets a gutter, links render as label (URL), the thematic break becomes a rule, and fenced code bodies are
indented two spaces.

Expanded-contract verification. Red/blue: with the expanded gate disabled the
bounded-preview case turns red, proving the tests catch a collapsed==expanded regression.
Real TUI with a 390-line plan: collapsed shows the header plus a preview of the plan body and a ... (N more lines) tail, and
Ctrl+O renders the full Markdown.

Two behaviors are pi-tui's design, not regressions:

  • Code fences keep the literal ``` characters — Markdown renders them via theme.codeBlockBorder() (pi-tui/dist/components/markdown.js:384) and colors them rather than drawing a background block.
  • Headings level 3 and deeper keep their ### prefix; only levels 1–2 hide it (markdown.js:350). Level 3+ still receives heading color and bold.

Impact

  • User-visible behavior:plan_ready output now goes through the TUI Markdown renderer instead of the plain-text fallback. Heading markers are consumed, list bullets and code fences are colored, inline code / bold / italic / link
    markers are consumed, tables are drawn with box characters, and fenced code bodies are indented. Nothing else changes.
  • Model-visible context / tools: none. No change to tool name, description, promptSnippet, promptGuidelines, parameters, or the text returned in content — only how the result is drawn.
  • Runtime / lifecycle: none. renderResult is pure presentation; terminate: true semantics and the plan-state commit are untouched.
  • Persisted config / data: none.
  • Compatibility / risk: low. plan_ready remains parent-only (already in CHILD_EXCLUDED_TOOL_NAMES, extensions/shared/child-session.ts:452), so the child-session drift guard is unaffected — its test passes.

Summary by CodeRabbit

  • New Features

    • Plan results now display completed plans with Markdown formatting and consistent visual styling.
    • Empty or missing plan content is shown with a muted fallback message.
    • Expanded and collapsed plan views provide consistent output.
  • Tests

    • Added coverage for headings, code blocks, rich content, large plans, plain text, Unicode, and empty results.

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Must-Fix:extensions/plan-mode/index.ts:455-462 的自定义 renderResult 忽略了 options.expanded,折叠态和展开态都会返回完整 Markdown。Pi 会把 expanded 状态传给自定义 renderer,但不会替它截断 renderer 返回的行;因此接近现有 48 KiB 上限的计划会在默认折叠态整段铺进 transcript,Ctrl+O 也不会产生任何折叠效果。新增测试 tests/extensions/plan-mode/result-rendering.test.ts:242-252 还把 collapsed/expanded 完全相同固定成了预期,这与 PR 中“Pi 已提供 expand/collapse,所以不需要第二层截断”的理由相矛盾。请在 expanded === false 时提供有界摘要/预览,在展开态再渲染完整 Markdown,并补一条长计划测试,证明默认输出行数有界且展开能恢复完整内容。

@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 07858baf-7af6-4723-b714-f6870c0ccccc

📥 Commits

Reviewing files that changed from the base of the PR and between 865f66e and bc0e564.

📒 Files selected for processing (2)
  • extensions/plan-mode/index.ts
  • tests/extensions/plan-mode/result-rendering.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The plan mode extension now renders completed plan_ready results as themed Markdown and shows a muted fallback when plan content is missing. New tests cover Markdown structures, unusual input, and expansion states.

Changes

Plan result rendering

Layer / File(s)Summary
Plan result rendering integration
extensions/plan-mode/index.ts
The plan_ready tool renders completed plans with the shared Markdown theme and displays missing content as muted text.
Rendering behavior validation
tests/extensions/plan-mode/result-rendering.test.ts
Tests cover tool registration, Markdown content, fallback handling, unusual input, code fences, and identical expanded or collapsed output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:⚪ Minimal · up to bc0e5

This change only renders finalized plans as Markdown in the TUI without altering plan content, tools, state, or runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers:tt-a1i

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly and concisely identifies the main change: rendering plan_ready output as Markdown in Plan Mode.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

plan_ready registered no renderResult, so the TUI fell back to its plain-text renderer and the recorded plan appeared as raw Markdown source. Render it with the Markdown component, the same treatment subagent results already get.
Validated with bun run check (format, lint, tsc) and bun run test (959 node:test cases and 30 vitest cases pass) on Linux.
Nine cases pin that the renderer is registered and that a realistic plan survives it: headings, ordered and nested lists, block quotes, tables, inline code and bold, links, three fenced code shapes, a thematic break, a 40K plan, and unicode.
Assertions strip ANSI first, because the Markdown component colors itself from the global theme rather than the theme passed to renderResult.
Verified red against the pre-fix code (all nine fail with "plan_ready must supply renderResult"), so these catch the regression rather than just documenting it.
Collapsed state was identical to expanded: the custom renderResult ignored
the expanded flag, so a plan near the 48KiB cap flooded the transcript and
Ctrl+O produced no visible change. Pi never truncates custom renderer
output; the renderer owns the collapsed/expanded contract (same pattern as
git-read and renderWaitResult).
Collapsed now renders one bounded line with the line count and an expand
hint via keyHint; expanded renders the full Markdown as before. Replaces
the old case that pinned collapsed == expanded with two cases: collapsed
stay bounded and expanded restores the full plan.
@Matt-qwq
Matt-qwqforce-pushed the fix/plan-ready-markdown-render branch from bc0e564 to 80a6e2cCompareAugust 30, 2026 10:41
CodeRabbit docstring coverage scoped to diff-touched functions requires
80%. Adds JSDoc to plan_ready renderResult (index.ts) and the stripAnsi,
loadPlanReady and renderPlan helpers (result-rendering.test.ts).
A bare line count made the collapsed result impossible to scan in a long
session. Collapsed now shows the header (Plan ready · N lines · expand
hint) followed by the first PLAN_PREVIEW_LINES (10) plan lines and a
... (N more lines) tail; the expand hint and tail disappear when the plan
fits the preview, matching bash/fallback and git-read conventions.
Tests: long-plan preview bounded with tail hidden, short plan without
hint, count boundaries at the cap (1/10/11 lines), trailing newline
semantics, narrow-width wrapping of unbroken lines, deterministic output,
and the expanded tail sentinel restore. Red-verified: disabling the
expanded gate fails the five collapsed cases.
@Matt-qwq

Copy link
Copy Markdown
Author

已按 P1 修复完毕:renderResult 现在遵循 expanded 契约——折叠态渲染有界预览(Plan ready · N lines · + 前 10 行内容 + ...(N more lines) 尾注),展开态渲染完整 Markdown。旧测试(断言折叠=展开)已替换为边界用例(行数边界1/10/11、尾换行、窄屏 wrap、幂等、末行 sentinel),红绿验证通过。请重新 review。

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

复审 exact head c39f119373ad1a44402a46418d46144a030559c4。这次已实现 expanded 分支和多行摘要,旧版折叠/展开完全相同的问题已有实质修复。渲染计划为 Markdown 的价值成立,也没有改变模型正文或 Plan Mode 生命周期。

Standards

0 项确认违规;沿用 Pi renderResult,无需新抽象。

Spec

仍有 1 项 P2:预览只限制源文本行,不限制终端换行后的屏幕行。真实 renderer 输入47000字符单行(小于48KiB上限),width=40,折叠态输出1176行,且没有展开提示。默认输出仍可能刷屏;详见行内。

验证:bun run check通过;result-rendering专项15/15通过;上述独立renderer复现通过。完整测试并行尝试遇到setup集成子进程超时,不能记为全套通过;此复现不依赖那个失败。未做安装后真实TUI验收。只需补实际渲染行的裁剪和长单行回归,不需要新增组件框架。

text +=
theme.fg("muted", " · ") + keyHint("app.tools.expand", "to expand");
}
for (const line of lines.slice(0, PLAN_PREVIEW_LINES)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 预览应限制渲染行,而不只是源文本行

这里 slice(0, PLAN_PREVIEW_LINES) 后仍返回会按终端宽度换行的 Text。合法输入 "a".repeat(47000) 只有一行,小于已有48KiB上限;实际 renderResult(...,{expanded:false},theme).render(40) 返回1176行,而且 lines.length === 1 使展开提示不出现。因而这条旧问题只修复了多行输入,长段落/minified内容仍刷屏。请按实际可见行约束折叠预览(并保留展开的完整内容),加一个长单行窄屏测试,断言总行数有界;当前窄屏测试只断言每行宽度。

@tt-a1i

Copy link
Copy Markdown
Collaborator

当前 head c39f119 已复审:旧 expanded 分支已修好,但47000字符单行在40列折叠态仍输出1176行,无展开提示。请按实际渲染行裁剪,详见 #293 (review) 。check和15项专项通过;未宣称全套或真实TUI通过。本轮未改代码或合并。

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Matt-qwq@tt-a1i
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(plan-mode): render plan_ready output as Markdown by Matt-qwq · Pull Request #293 · openpi-dev/openpi · GitHub
Skip to content

fix(plan-mode): render plan_ready output as Markdown - #293

Open
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render
Open

fix(plan-mode): render plan_ready output as Markdown#293
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render

Conversation

@Matt-qwq

@Matt-qwqMatt-qwq commented Aug 29, 2026

Copy link
Copy Markdown

Problem

plan_ready renders the finalized plan as raw Markdown source in the TUI.

extensions/plan-mode/index.ts registers plan_ready with only an execute handler and no renderResult, so the TUI falls back to its default plain-text renderer: headings, lists, and code fences appear as literal #, -, and ```
characters instead of rendered text.

Affects every Plan Mode user — the plan is the one artifact Plan Mode exists to produce, and it is meant to be read, not inspected as source.

No existing issue tracks this. I searched plan_ready, renderResult, and Markdown-rendering across open and closed issues in openpi-dev/openpi; the closest hits (#28, #27, #18, #105, #101, #96, #93, #67, #40) are all about other
subsystems.

Value

Every other extension tool that returns human-readable prose already supplies a renderer — there are 19 renderResult implementations across goal, tasks, git-read, file-search, workflows, subagents, ask-user,
background-terminals, and file-mutation-display. plan_ready is the outlier. Rendering it makes Plan Mode's output consistent with the rest of OpenPI and lets the user read the plan without decoding Markdown syntax.

Approach

Add renderResult to the plan_ready registration, rendering result.details.plan with the Markdown component from @earendil-works/pi-tui using getMarkdownTheme() — the same treatment renderWaitResult() gives subagent results
(extensions/subagents/src/ui/wait-result.ts:59).

Two deliberate choices:

  • Collapsed state shows a bounded preview. Pi passes the expanded flag to custom renderers but never truncates their output — the renderer owns the collapsed/expanded contract (same pattern as git-read and renderWaitResult). Collapsed renders Plan ready · N lines · <to expand> plus the first 10 plan lines and a ... (N more lines) tail; the hint and tail disappear when the plan fits the preview. Only the expanded state renders the full Markdown. Without this, a plan near the 48 KiB cap floods the transcript and Ctrl+O produces no visible change.
  • No re-sanitizing.execute already stores sanitizeTerminalText(params.plan), so details.plan is clean on the way in.

Validation

Ran on WSL2 (Fedora 44), Node v22.23.1, bun 1.3.14:

bun run check # biome format (257 files), biome lint --error-on-warnings, tsc --noEmit
bun run test # node:test 968 cases: 967 pass / 0 fail / 1 skipped
# vitest 1 file: 30 tests passed

bun run test grows from 959 to 968 cases — the 9 in tests/extensions/plan-mode/result-rendering.test.ts. Those were verified red against the pre-fix code: with renderResult removed, all nine fail with plan_ready must supply renderResult, so they catch the regression rather than document it.

Manual TUI check. Loaded the checkout with pi install, ran /plan, and called plan_ready with a plan exercising headings, ordered and nested lists, a block quote, a table, inline code, bold, italics, links, three fenced code shapes,
and a thematic break. In the TUI: heading markers are consumed, the table renders with box-drawing characters, the quote gets a gutter, links render as label (URL), the thematic break becomes a rule, and fenced code bodies are
indented two spaces.

Expanded-contract verification. Red/blue: with the expanded gate disabled the
bounded-preview case turns red, proving the tests catch a collapsed==expanded regression.
Real TUI with a 390-line plan: collapsed shows the header plus a preview of the plan body and a ... (N more lines) tail, and
Ctrl+O renders the full Markdown.

Two behaviors are pi-tui's design, not regressions:

  • Code fences keep the literal ``` characters — Markdown renders them via theme.codeBlockBorder() (pi-tui/dist/components/markdown.js:384) and colors them rather than drawing a background block.
  • Headings level 3 and deeper keep their ### prefix; only levels 1–2 hide it (markdown.js:350). Level 3+ still receives heading color and bold.

Impact

  • User-visible behavior:plan_ready output now goes through the TUI Markdown renderer instead of the plain-text fallback. Heading markers are consumed, list bullets and code fences are colored, inline code / bold / italic / link
    markers are consumed, tables are drawn with box characters, and fenced code bodies are indented. Nothing else changes.
  • Model-visible context / tools: none. No change to tool name, description, promptSnippet, promptGuidelines, parameters, or the text returned in content — only how the result is drawn.
  • Runtime / lifecycle: none. renderResult is pure presentation; terminate: true semantics and the plan-state commit are untouched.
  • Persisted config / data: none.
  • Compatibility / risk: low. plan_ready remains parent-only (already in CHILD_EXCLUDED_TOOL_NAMES, extensions/shared/child-session.ts:452), so the child-session drift guard is unaffected — its test passes.

Summary by CodeRabbit

  • New Features

    • Plan results now display completed plans with Markdown formatting and consistent visual styling.
    • Empty or missing plan content is shown with a muted fallback message.
    • Expanded and collapsed plan views provide consistent output.
  • Tests

    • Added coverage for headings, code blocks, rich content, large plans, plain text, Unicode, and empty results.

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Must-Fix:extensions/plan-mode/index.ts:455-462 的自定义 renderResult 忽略了 options.expanded,折叠态和展开态都会返回完整 Markdown。Pi 会把 expanded 状态传给自定义 renderer,但不会替它截断 renderer 返回的行;因此接近现有 48 KiB 上限的计划会在默认折叠态整段铺进 transcript,Ctrl+O 也不会产生任何折叠效果。新增测试 tests/extensions/plan-mode/result-rendering.test.ts:242-252 还把 collapsed/expanded 完全相同固定成了预期,这与 PR 中“Pi 已提供 expand/collapse,所以不需要第二层截断”的理由相矛盾。请在 expanded === false 时提供有界摘要/预览,在展开态再渲染完整 Markdown,并补一条长计划测试,证明默认输出行数有界且展开能恢复完整内容。

@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 07858baf-7af6-4723-b714-f6870c0ccccc

📥 Commits

Reviewing files that changed from the base of the PR and between 865f66e and bc0e564.

📒 Files selected for processing (2)
  • extensions/plan-mode/index.ts
  • tests/extensions/plan-mode/result-rendering.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The plan mode extension now renders completed plan_ready results as themed Markdown and shows a muted fallback when plan content is missing. New tests cover Markdown structures, unusual input, and expansion states.

Changes

Plan result rendering

Layer / File(s)Summary
Plan result rendering integration
extensions/plan-mode/index.ts
The plan_ready tool renders completed plans with the shared Markdown theme and displays missing content as muted text.
Rendering behavior validation
tests/extensions/plan-mode/result-rendering.test.ts
Tests cover tool registration, Markdown content, fallback handling, unusual input, code fences, and identical expanded or collapsed output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:⚪ Minimal · up to bc0e5

This change only renders finalized plans as Markdown in the TUI without altering plan content, tools, state, or runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers:tt-a1i

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly and concisely identifies the main change: rendering plan_ready output as Markdown in Plan Mode.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

plan_ready registered no renderResult, so the TUI fell back to its plain-text renderer and the recorded plan appeared as raw Markdown source. Render it with the Markdown component, the same treatment subagent results already get.
Validated with bun run check (format, lint, tsc) and bun run test (959 node:test cases and 30 vitest cases pass) on Linux.
Nine cases pin that the renderer is registered and that a realistic plan survives it: headings, ordered and nested lists, block quotes, tables, inline code and bold, links, three fenced code shapes, a thematic break, a 40K plan, and unicode.
Assertions strip ANSI first, because the Markdown component colors itself from the global theme rather than the theme passed to renderResult.
Verified red against the pre-fix code (all nine fail with "plan_ready must supply renderResult"), so these catch the regression rather than just documenting it.
Collapsed state was identical to expanded: the custom renderResult ignored
the expanded flag, so a plan near the 48KiB cap flooded the transcript and
Ctrl+O produced no visible change. Pi never truncates custom renderer
output; the renderer owns the collapsed/expanded contract (same pattern as
git-read and renderWaitResult).
Collapsed now renders one bounded line with the line count and an expand
hint via keyHint; expanded renders the full Markdown as before. Replaces
the old case that pinned collapsed == expanded with two cases: collapsed
stay bounded and expanded restores the full plan.
@Matt-qwq
Matt-qwqforce-pushed the fix/plan-ready-markdown-render branch from bc0e564 to 80a6e2cCompareAugust 30, 2026 10:41
CodeRabbit docstring coverage scoped to diff-touched functions requires
80%. Adds JSDoc to plan_ready renderResult (index.ts) and the stripAnsi,
loadPlanReady and renderPlan helpers (result-rendering.test.ts).
A bare line count made the collapsed result impossible to scan in a long
session. Collapsed now shows the header (Plan ready · N lines · expand
hint) followed by the first PLAN_PREVIEW_LINES (10) plan lines and a
... (N more lines) tail; the expand hint and tail disappear when the plan
fits the preview, matching bash/fallback and git-read conventions.
Tests: long-plan preview bounded with tail hidden, short plan without
hint, count boundaries at the cap (1/10/11 lines), trailing newline
semantics, narrow-width wrapping of unbroken lines, deterministic output,
and the expanded tail sentinel restore. Red-verified: disabling the
expanded gate fails the five collapsed cases.
@Matt-qwq

Copy link
Copy Markdown
Author

已按 P1 修复完毕:renderResult 现在遵循 expanded 契约——折叠态渲染有界预览(Plan ready · N lines · + 前 10 行内容 + ...(N more lines) 尾注),展开态渲染完整 Markdown。旧测试(断言折叠=展开)已替换为边界用例(行数边界1/10/11、尾换行、窄屏 wrap、幂等、末行 sentinel),红绿验证通过。请重新 review。

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

复审 exact head c39f119373ad1a44402a46418d46144a030559c4。这次已实现 expanded 分支和多行摘要,旧版折叠/展开完全相同的问题已有实质修复。渲染计划为 Markdown 的价值成立,也没有改变模型正文或 Plan Mode 生命周期。

Standards

0 项确认违规;沿用 Pi renderResult,无需新抽象。

Spec

仍有 1 项 P2:预览只限制源文本行,不限制终端换行后的屏幕行。真实 renderer 输入47000字符单行(小于48KiB上限),width=40,折叠态输出1176行,且没有展开提示。默认输出仍可能刷屏;详见行内。

验证:bun run check通过;result-rendering专项15/15通过;上述独立renderer复现通过。完整测试并行尝试遇到setup集成子进程超时,不能记为全套通过;此复现不依赖那个失败。未做安装后真实TUI验收。只需补实际渲染行的裁剪和长单行回归,不需要新增组件框架。

text +=
theme.fg("muted", " · ") + keyHint("app.tools.expand", "to expand");
}
for (const line of lines.slice(0, PLAN_PREVIEW_LINES)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 预览应限制渲染行,而不只是源文本行

这里 slice(0, PLAN_PREVIEW_LINES) 后仍返回会按终端宽度换行的 Text。合法输入 "a".repeat(47000) 只有一行,小于已有48KiB上限;实际 renderResult(...,{expanded:false},theme).render(40) 返回1176行,而且 lines.length === 1 使展开提示不出现。因而这条旧问题只修复了多行输入,长段落/minified内容仍刷屏。请按实际可见行约束折叠预览(并保留展开的完整内容),加一个长单行窄屏测试,断言总行数有界;当前窄屏测试只断言每行宽度。

@tt-a1i

Copy link
Copy Markdown
Collaborator

当前 head c39f119 已复审:旧 expanded 分支已修好,但47000字符单行在40列折叠态仍输出1176行,无展开提示。请按实际渲染行裁剪,详见 #293 (review) 。check和15项专项通过;未宣称全套或真实TUI通过。本轮未改代码或合并。

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Matt-qwq@tt-a1i
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(plan-mode): render plan_ready output as Markdown by Matt-qwq · Pull Request #293 · openpi-dev/openpi · GitHub
Skip to content

fix(plan-mode): render plan_ready output as Markdown - #293

Open
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render
Open

fix(plan-mode): render plan_ready output as Markdown#293
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render

Conversation

@Matt-qwq

@Matt-qwqMatt-qwq commented Aug 29, 2026

Copy link
Copy Markdown

Problem

plan_ready renders the finalized plan as raw Markdown source in the TUI.

extensions/plan-mode/index.ts registers plan_ready with only an execute handler and no renderResult, so the TUI falls back to its default plain-text renderer: headings, lists, and code fences appear as literal #, -, and ```
characters instead of rendered text.

Affects every Plan Mode user — the plan is the one artifact Plan Mode exists to produce, and it is meant to be read, not inspected as source.

No existing issue tracks this. I searched plan_ready, renderResult, and Markdown-rendering across open and closed issues in openpi-dev/openpi; the closest hits (#28, #27, #18, #105, #101, #96, #93, #67, #40) are all about other
subsystems.

Value

Every other extension tool that returns human-readable prose already supplies a renderer — there are 19 renderResult implementations across goal, tasks, git-read, file-search, workflows, subagents, ask-user,
background-terminals, and file-mutation-display. plan_ready is the outlier. Rendering it makes Plan Mode's output consistent with the rest of OpenPI and lets the user read the plan without decoding Markdown syntax.

Approach

Add renderResult to the plan_ready registration, rendering result.details.plan with the Markdown component from @earendil-works/pi-tui using getMarkdownTheme() — the same treatment renderWaitResult() gives subagent results
(extensions/subagents/src/ui/wait-result.ts:59).

Two deliberate choices:

  • Collapsed state shows a bounded preview. Pi passes the expanded flag to custom renderers but never truncates their output — the renderer owns the collapsed/expanded contract (same pattern as git-read and renderWaitResult). Collapsed renders Plan ready · N lines · <to expand> plus the first 10 plan lines and a ... (N more lines) tail; the hint and tail disappear when the plan fits the preview. Only the expanded state renders the full Markdown. Without this, a plan near the 48 KiB cap floods the transcript and Ctrl+O produces no visible change.
  • No re-sanitizing.execute already stores sanitizeTerminalText(params.plan), so details.plan is clean on the way in.

Validation

Ran on WSL2 (Fedora 44), Node v22.23.1, bun 1.3.14:

bun run check # biome format (257 files), biome lint --error-on-warnings, tsc --noEmit
bun run test # node:test 968 cases: 967 pass / 0 fail / 1 skipped
# vitest 1 file: 30 tests passed

bun run test grows from 959 to 968 cases — the 9 in tests/extensions/plan-mode/result-rendering.test.ts. Those were verified red against the pre-fix code: with renderResult removed, all nine fail with plan_ready must supply renderResult, so they catch the regression rather than document it.

Manual TUI check. Loaded the checkout with pi install, ran /plan, and called plan_ready with a plan exercising headings, ordered and nested lists, a block quote, a table, inline code, bold, italics, links, three fenced code shapes,
and a thematic break. In the TUI: heading markers are consumed, the table renders with box-drawing characters, the quote gets a gutter, links render as label (URL), the thematic break becomes a rule, and fenced code bodies are
indented two spaces.

Expanded-contract verification. Red/blue: with the expanded gate disabled the
bounded-preview case turns red, proving the tests catch a collapsed==expanded regression.
Real TUI with a 390-line plan: collapsed shows the header plus a preview of the plan body and a ... (N more lines) tail, and
Ctrl+O renders the full Markdown.

Two behaviors are pi-tui's design, not regressions:

  • Code fences keep the literal ``` characters — Markdown renders them via theme.codeBlockBorder() (pi-tui/dist/components/markdown.js:384) and colors them rather than drawing a background block.
  • Headings level 3 and deeper keep their ### prefix; only levels 1–2 hide it (markdown.js:350). Level 3+ still receives heading color and bold.

Impact

  • User-visible behavior:plan_ready output now goes through the TUI Markdown renderer instead of the plain-text fallback. Heading markers are consumed, list bullets and code fences are colored, inline code / bold / italic / link
    markers are consumed, tables are drawn with box characters, and fenced code bodies are indented. Nothing else changes.
  • Model-visible context / tools: none. No change to tool name, description, promptSnippet, promptGuidelines, parameters, or the text returned in content — only how the result is drawn.
  • Runtime / lifecycle: none. renderResult is pure presentation; terminate: true semantics and the plan-state commit are untouched.
  • Persisted config / data: none.
  • Compatibility / risk: low. plan_ready remains parent-only (already in CHILD_EXCLUDED_TOOL_NAMES, extensions/shared/child-session.ts:452), so the child-session drift guard is unaffected — its test passes.

Summary by CodeRabbit

  • New Features

    • Plan results now display completed plans with Markdown formatting and consistent visual styling.
    • Empty or missing plan content is shown with a muted fallback message.
    • Expanded and collapsed plan views provide consistent output.
  • Tests

    • Added coverage for headings, code blocks, rich content, large plans, plain text, Unicode, and empty results.

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Must-Fix:extensions/plan-mode/index.ts:455-462 的自定义 renderResult 忽略了 options.expanded,折叠态和展开态都会返回完整 Markdown。Pi 会把 expanded 状态传给自定义 renderer,但不会替它截断 renderer 返回的行;因此接近现有 48 KiB 上限的计划会在默认折叠态整段铺进 transcript,Ctrl+O 也不会产生任何折叠效果。新增测试 tests/extensions/plan-mode/result-rendering.test.ts:242-252 还把 collapsed/expanded 完全相同固定成了预期,这与 PR 中“Pi 已提供 expand/collapse,所以不需要第二层截断”的理由相矛盾。请在 expanded === false 时提供有界摘要/预览,在展开态再渲染完整 Markdown,并补一条长计划测试,证明默认输出行数有界且展开能恢复完整内容。

@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 07858baf-7af6-4723-b714-f6870c0ccccc

📥 Commits

Reviewing files that changed from the base of the PR and between 865f66e and bc0e564.

📒 Files selected for processing (2)
  • extensions/plan-mode/index.ts
  • tests/extensions/plan-mode/result-rendering.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The plan mode extension now renders completed plan_ready results as themed Markdown and shows a muted fallback when plan content is missing. New tests cover Markdown structures, unusual input, and expansion states.

Changes

Plan result rendering

Layer / File(s)Summary
Plan result rendering integration
extensions/plan-mode/index.ts
The plan_ready tool renders completed plans with the shared Markdown theme and displays missing content as muted text.
Rendering behavior validation
tests/extensions/plan-mode/result-rendering.test.ts
Tests cover tool registration, Markdown content, fallback handling, unusual input, code fences, and identical expanded or collapsed output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:⚪ Minimal · up to bc0e5

This change only renders finalized plans as Markdown in the TUI without altering plan content, tools, state, or runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers:tt-a1i

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly and concisely identifies the main change: rendering plan_ready output as Markdown in Plan Mode.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

plan_ready registered no renderResult, so the TUI fell back to its plain-text renderer and the recorded plan appeared as raw Markdown source. Render it with the Markdown component, the same treatment subagent results already get.
Validated with bun run check (format, lint, tsc) and bun run test (959 node:test cases and 30 vitest cases pass) on Linux.
Nine cases pin that the renderer is registered and that a realistic plan survives it: headings, ordered and nested lists, block quotes, tables, inline code and bold, links, three fenced code shapes, a thematic break, a 40K plan, and unicode.
Assertions strip ANSI first, because the Markdown component colors itself from the global theme rather than the theme passed to renderResult.
Verified red against the pre-fix code (all nine fail with "plan_ready must supply renderResult"), so these catch the regression rather than just documenting it.
Collapsed state was identical to expanded: the custom renderResult ignored
the expanded flag, so a plan near the 48KiB cap flooded the transcript and
Ctrl+O produced no visible change. Pi never truncates custom renderer
output; the renderer owns the collapsed/expanded contract (same pattern as
git-read and renderWaitResult).
Collapsed now renders one bounded line with the line count and an expand
hint via keyHint; expanded renders the full Markdown as before. Replaces
the old case that pinned collapsed == expanded with two cases: collapsed
stay bounded and expanded restores the full plan.
@Matt-qwq
Matt-qwqforce-pushed the fix/plan-ready-markdown-render branch from bc0e564 to 80a6e2cCompareAugust 30, 2026 10:41
CodeRabbit docstring coverage scoped to diff-touched functions requires
80%. Adds JSDoc to plan_ready renderResult (index.ts) and the stripAnsi,
loadPlanReady and renderPlan helpers (result-rendering.test.ts).
A bare line count made the collapsed result impossible to scan in a long
session. Collapsed now shows the header (Plan ready · N lines · expand
hint) followed by the first PLAN_PREVIEW_LINES (10) plan lines and a
... (N more lines) tail; the expand hint and tail disappear when the plan
fits the preview, matching bash/fallback and git-read conventions.
Tests: long-plan preview bounded with tail hidden, short plan without
hint, count boundaries at the cap (1/10/11 lines), trailing newline
semantics, narrow-width wrapping of unbroken lines, deterministic output,
and the expanded tail sentinel restore. Red-verified: disabling the
expanded gate fails the five collapsed cases.
@Matt-qwq

Copy link
Copy Markdown
Author

已按 P1 修复完毕:renderResult 现在遵循 expanded 契约——折叠态渲染有界预览(Plan ready · N lines · + 前 10 行内容 + ...(N more lines) 尾注),展开态渲染完整 Markdown。旧测试(断言折叠=展开)已替换为边界用例(行数边界1/10/11、尾换行、窄屏 wrap、幂等、末行 sentinel),红绿验证通过。请重新 review。

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

复审 exact head c39f119373ad1a44402a46418d46144a030559c4。这次已实现 expanded 分支和多行摘要,旧版折叠/展开完全相同的问题已有实质修复。渲染计划为 Markdown 的价值成立,也没有改变模型正文或 Plan Mode 生命周期。

Standards

0 项确认违规;沿用 Pi renderResult,无需新抽象。

Spec

仍有 1 项 P2:预览只限制源文本行,不限制终端换行后的屏幕行。真实 renderer 输入47000字符单行(小于48KiB上限),width=40,折叠态输出1176行,且没有展开提示。默认输出仍可能刷屏;详见行内。

验证:bun run check通过;result-rendering专项15/15通过;上述独立renderer复现通过。完整测试并行尝试遇到setup集成子进程超时,不能记为全套通过;此复现不依赖那个失败。未做安装后真实TUI验收。只需补实际渲染行的裁剪和长单行回归,不需要新增组件框架。

text +=
theme.fg("muted", " · ") + keyHint("app.tools.expand", "to expand");
}
for (const line of lines.slice(0, PLAN_PREVIEW_LINES)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 预览应限制渲染行,而不只是源文本行

这里 slice(0, PLAN_PREVIEW_LINES) 后仍返回会按终端宽度换行的 Text。合法输入 "a".repeat(47000) 只有一行,小于已有48KiB上限;实际 renderResult(...,{expanded:false},theme).render(40) 返回1176行,而且 lines.length === 1 使展开提示不出现。因而这条旧问题只修复了多行输入,长段落/minified内容仍刷屏。请按实际可见行约束折叠预览(并保留展开的完整内容),加一个长单行窄屏测试,断言总行数有界;当前窄屏测试只断言每行宽度。

@tt-a1i

Copy link
Copy Markdown
Collaborator

当前 head c39f119 已复审:旧 expanded 分支已修好,但47000字符单行在40列折叠态仍输出1176行,无展开提示。请按实际渲染行裁剪,详见 #293 (review) 。check和15项专项通过;未宣称全套或真实TUI通过。本轮未改代码或合并。

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Matt-qwq@tt-a1i
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(plan-mode): render plan_ready output as Markdown by Matt-qwq · Pull Request #293 · openpi-dev/openpi · GitHub
Skip to content

fix(plan-mode): render plan_ready output as Markdown - #293

Open
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render
Open

fix(plan-mode): render plan_ready output as Markdown#293
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render

Conversation

@Matt-qwq

@Matt-qwqMatt-qwq commented Aug 29, 2026

Copy link
Copy Markdown

Problem

plan_ready renders the finalized plan as raw Markdown source in the TUI.

extensions/plan-mode/index.ts registers plan_ready with only an execute handler and no renderResult, so the TUI falls back to its default plain-text renderer: headings, lists, and code fences appear as literal #, -, and ```
characters instead of rendered text.

Affects every Plan Mode user — the plan is the one artifact Plan Mode exists to produce, and it is meant to be read, not inspected as source.

No existing issue tracks this. I searched plan_ready, renderResult, and Markdown-rendering across open and closed issues in openpi-dev/openpi; the closest hits (#28, #27, #18, #105, #101, #96, #93, #67, #40) are all about other
subsystems.

Value

Every other extension tool that returns human-readable prose already supplies a renderer — there are 19 renderResult implementations across goal, tasks, git-read, file-search, workflows, subagents, ask-user,
background-terminals, and file-mutation-display. plan_ready is the outlier. Rendering it makes Plan Mode's output consistent with the rest of OpenPI and lets the user read the plan without decoding Markdown syntax.

Approach

Add renderResult to the plan_ready registration, rendering result.details.plan with the Markdown component from @earendil-works/pi-tui using getMarkdownTheme() — the same treatment renderWaitResult() gives subagent results
(extensions/subagents/src/ui/wait-result.ts:59).

Two deliberate choices:

  • Collapsed state shows a bounded preview. Pi passes the expanded flag to custom renderers but never truncates their output — the renderer owns the collapsed/expanded contract (same pattern as git-read and renderWaitResult). Collapsed renders Plan ready · N lines · <to expand> plus the first 10 plan lines and a ... (N more lines) tail; the hint and tail disappear when the plan fits the preview. Only the expanded state renders the full Markdown. Without this, a plan near the 48 KiB cap floods the transcript and Ctrl+O produces no visible change.
  • No re-sanitizing.execute already stores sanitizeTerminalText(params.plan), so details.plan is clean on the way in.

Validation

Ran on WSL2 (Fedora 44), Node v22.23.1, bun 1.3.14:

bun run check # biome format (257 files), biome lint --error-on-warnings, tsc --noEmit
bun run test # node:test 968 cases: 967 pass / 0 fail / 1 skipped
# vitest 1 file: 30 tests passed

bun run test grows from 959 to 968 cases — the 9 in tests/extensions/plan-mode/result-rendering.test.ts. Those were verified red against the pre-fix code: with renderResult removed, all nine fail with plan_ready must supply renderResult, so they catch the regression rather than document it.

Manual TUI check. Loaded the checkout with pi install, ran /plan, and called plan_ready with a plan exercising headings, ordered and nested lists, a block quote, a table, inline code, bold, italics, links, three fenced code shapes,
and a thematic break. In the TUI: heading markers are consumed, the table renders with box-drawing characters, the quote gets a gutter, links render as label (URL), the thematic break becomes a rule, and fenced code bodies are
indented two spaces.

Expanded-contract verification. Red/blue: with the expanded gate disabled the
bounded-preview case turns red, proving the tests catch a collapsed==expanded regression.
Real TUI with a 390-line plan: collapsed shows the header plus a preview of the plan body and a ... (N more lines) tail, and
Ctrl+O renders the full Markdown.

Two behaviors are pi-tui's design, not regressions:

  • Code fences keep the literal ``` characters — Markdown renders them via theme.codeBlockBorder() (pi-tui/dist/components/markdown.js:384) and colors them rather than drawing a background block.
  • Headings level 3 and deeper keep their ### prefix; only levels 1–2 hide it (markdown.js:350). Level 3+ still receives heading color and bold.

Impact

  • User-visible behavior:plan_ready output now goes through the TUI Markdown renderer instead of the plain-text fallback. Heading markers are consumed, list bullets and code fences are colored, inline code / bold / italic / link
    markers are consumed, tables are drawn with box characters, and fenced code bodies are indented. Nothing else changes.
  • Model-visible context / tools: none. No change to tool name, description, promptSnippet, promptGuidelines, parameters, or the text returned in content — only how the result is drawn.
  • Runtime / lifecycle: none. renderResult is pure presentation; terminate: true semantics and the plan-state commit are untouched.
  • Persisted config / data: none.
  • Compatibility / risk: low. plan_ready remains parent-only (already in CHILD_EXCLUDED_TOOL_NAMES, extensions/shared/child-session.ts:452), so the child-session drift guard is unaffected — its test passes.

Summary by CodeRabbit

  • New Features

    • Plan results now display completed plans with Markdown formatting and consistent visual styling.
    • Empty or missing plan content is shown with a muted fallback message.
    • Expanded and collapsed plan views provide consistent output.
  • Tests

    • Added coverage for headings, code blocks, rich content, large plans, plain text, Unicode, and empty results.

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Must-Fix:extensions/plan-mode/index.ts:455-462 的自定义 renderResult 忽略了 options.expanded,折叠态和展开态都会返回完整 Markdown。Pi 会把 expanded 状态传给自定义 renderer,但不会替它截断 renderer 返回的行;因此接近现有 48 KiB 上限的计划会在默认折叠态整段铺进 transcript,Ctrl+O 也不会产生任何折叠效果。新增测试 tests/extensions/plan-mode/result-rendering.test.ts:242-252 还把 collapsed/expanded 完全相同固定成了预期,这与 PR 中“Pi 已提供 expand/collapse,所以不需要第二层截断”的理由相矛盾。请在 expanded === false 时提供有界摘要/预览,在展开态再渲染完整 Markdown,并补一条长计划测试,证明默认输出行数有界且展开能恢复完整内容。

@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 07858baf-7af6-4723-b714-f6870c0ccccc

📥 Commits

Reviewing files that changed from the base of the PR and between 865f66e and bc0e564.

📒 Files selected for processing (2)
  • extensions/plan-mode/index.ts
  • tests/extensions/plan-mode/result-rendering.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The plan mode extension now renders completed plan_ready results as themed Markdown and shows a muted fallback when plan content is missing. New tests cover Markdown structures, unusual input, and expansion states.

Changes

Plan result rendering

Layer / File(s)Summary
Plan result rendering integration
extensions/plan-mode/index.ts
The plan_ready tool renders completed plans with the shared Markdown theme and displays missing content as muted text.
Rendering behavior validation
tests/extensions/plan-mode/result-rendering.test.ts
Tests cover tool registration, Markdown content, fallback handling, unusual input, code fences, and identical expanded or collapsed output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:⚪ Minimal · up to bc0e5

This change only renders finalized plans as Markdown in the TUI without altering plan content, tools, state, or runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers:tt-a1i

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly and concisely identifies the main change: rendering plan_ready output as Markdown in Plan Mode.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

plan_ready registered no renderResult, so the TUI fell back to its plain-text renderer and the recorded plan appeared as raw Markdown source. Render it with the Markdown component, the same treatment subagent results already get.
Validated with bun run check (format, lint, tsc) and bun run test (959 node:test cases and 30 vitest cases pass) on Linux.
Nine cases pin that the renderer is registered and that a realistic plan survives it: headings, ordered and nested lists, block quotes, tables, inline code and bold, links, three fenced code shapes, a thematic break, a 40K plan, and unicode.
Assertions strip ANSI first, because the Markdown component colors itself from the global theme rather than the theme passed to renderResult.
Verified red against the pre-fix code (all nine fail with "plan_ready must supply renderResult"), so these catch the regression rather than just documenting it.
Collapsed state was identical to expanded: the custom renderResult ignored
the expanded flag, so a plan near the 48KiB cap flooded the transcript and
Ctrl+O produced no visible change. Pi never truncates custom renderer
output; the renderer owns the collapsed/expanded contract (same pattern as
git-read and renderWaitResult).
Collapsed now renders one bounded line with the line count and an expand
hint via keyHint; expanded renders the full Markdown as before. Replaces
the old case that pinned collapsed == expanded with two cases: collapsed
stay bounded and expanded restores the full plan.
@Matt-qwq
Matt-qwqforce-pushed the fix/plan-ready-markdown-render branch from bc0e564 to 80a6e2cCompareAugust 30, 2026 10:41
CodeRabbit docstring coverage scoped to diff-touched functions requires
80%. Adds JSDoc to plan_ready renderResult (index.ts) and the stripAnsi,
loadPlanReady and renderPlan helpers (result-rendering.test.ts).
A bare line count made the collapsed result impossible to scan in a long
session. Collapsed now shows the header (Plan ready · N lines · expand
hint) followed by the first PLAN_PREVIEW_LINES (10) plan lines and a
... (N more lines) tail; the expand hint and tail disappear when the plan
fits the preview, matching bash/fallback and git-read conventions.
Tests: long-plan preview bounded with tail hidden, short plan without
hint, count boundaries at the cap (1/10/11 lines), trailing newline
semantics, narrow-width wrapping of unbroken lines, deterministic output,
and the expanded tail sentinel restore. Red-verified: disabling the
expanded gate fails the five collapsed cases.
@Matt-qwq

Copy link
Copy Markdown
Author

已按 P1 修复完毕:renderResult 现在遵循 expanded 契约——折叠态渲染有界预览(Plan ready · N lines · + 前 10 行内容 + ...(N more lines) 尾注),展开态渲染完整 Markdown。旧测试(断言折叠=展开)已替换为边界用例(行数边界1/10/11、尾换行、窄屏 wrap、幂等、末行 sentinel),红绿验证通过。请重新 review。

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

复审 exact head c39f119373ad1a44402a46418d46144a030559c4。这次已实现 expanded 分支和多行摘要,旧版折叠/展开完全相同的问题已有实质修复。渲染计划为 Markdown 的价值成立,也没有改变模型正文或 Plan Mode 生命周期。

Standards

0 项确认违规;沿用 Pi renderResult,无需新抽象。

Spec

仍有 1 项 P2:预览只限制源文本行,不限制终端换行后的屏幕行。真实 renderer 输入47000字符单行(小于48KiB上限),width=40,折叠态输出1176行,且没有展开提示。默认输出仍可能刷屏;详见行内。

验证:bun run check通过;result-rendering专项15/15通过;上述独立renderer复现通过。完整测试并行尝试遇到setup集成子进程超时,不能记为全套通过;此复现不依赖那个失败。未做安装后真实TUI验收。只需补实际渲染行的裁剪和长单行回归,不需要新增组件框架。

text +=
theme.fg("muted", " · ") + keyHint("app.tools.expand", "to expand");
}
for (const line of lines.slice(0, PLAN_PREVIEW_LINES)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 预览应限制渲染行,而不只是源文本行

这里 slice(0, PLAN_PREVIEW_LINES) 后仍返回会按终端宽度换行的 Text。合法输入 "a".repeat(47000) 只有一行,小于已有48KiB上限;实际 renderResult(...,{expanded:false},theme).render(40) 返回1176行,而且 lines.length === 1 使展开提示不出现。因而这条旧问题只修复了多行输入,长段落/minified内容仍刷屏。请按实际可见行约束折叠预览(并保留展开的完整内容),加一个长单行窄屏测试,断言总行数有界;当前窄屏测试只断言每行宽度。

@tt-a1i

Copy link
Copy Markdown
Collaborator

当前 head c39f119 已复审:旧 expanded 分支已修好,但47000字符单行在40列折叠态仍输出1176行,无展开提示。请按实际渲染行裁剪,详见 #293 (review) 。check和15项专项通过;未宣称全套或真实TUI通过。本轮未改代码或合并。

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Matt-qwq@tt-a1i
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fix(plan-mode): render plan_ready output as Markdown by Matt-qwq · Pull Request #293 · openpi-dev/openpi · GitHub
Skip to content

fix(plan-mode): render plan_ready output as Markdown - #293

Open
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render
Open

fix(plan-mode): render plan_ready output as Markdown#293
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render

Conversation

@Matt-qwq

@Matt-qwqMatt-qwq commented Aug 29, 2026

Copy link
Copy Markdown

Problem

plan_ready renders the finalized plan as raw Markdown source in the TUI.

extensions/plan-mode/index.ts registers plan_ready with only an execute handler and no renderResult, so the TUI falls back to its default plain-text renderer: headings, lists, and code fences appear as literal #, -, and ```
characters instead of rendered text.

Affects every Plan Mode user — the plan is the one artifact Plan Mode exists to produce, and it is meant to be read, not inspected as source.

No existing issue tracks this. I searched plan_ready, renderResult, and Markdown-rendering across open and closed issues in openpi-dev/openpi; the closest hits (#28, #27, #18, #105, #101, #96, #93, #67, #40) are all about other
subsystems.

Value

Every other extension tool that returns human-readable prose already supplies a renderer — there are 19 renderResult implementations across goal, tasks, git-read, file-search, workflows, subagents, ask-user,
background-terminals, and file-mutation-display. plan_ready is the outlier. Rendering it makes Plan Mode's output consistent with the rest of OpenPI and lets the user read the plan without decoding Markdown syntax.

Approach

Add renderResult to the plan_ready registration, rendering result.details.plan with the Markdown component from @earendil-works/pi-tui using getMarkdownTheme() — the same treatment renderWaitResult() gives subagent results
(extensions/subagents/src/ui/wait-result.ts:59).

Two deliberate choices:

  • Collapsed state shows a bounded preview. Pi passes the expanded flag to custom renderers but never truncates their output — the renderer owns the collapsed/expanded contract (same pattern as git-read and renderWaitResult). Collapsed renders Plan ready · N lines · <to expand> plus the first 10 plan lines and a ... (N more lines) tail; the hint and tail disappear when the plan fits the preview. Only the expanded state renders the full Markdown. Without this, a plan near the 48 KiB cap floods the transcript and Ctrl+O produces no visible change.
  • No re-sanitizing.execute already stores sanitizeTerminalText(params.plan), so details.plan is clean on the way in.

Validation

Ran on WSL2 (Fedora 44), Node v22.23.1, bun 1.3.14:

bun run check # biome format (257 files), biome lint --error-on-warnings, tsc --noEmit
bun run test # node:test 968 cases: 967 pass / 0 fail / 1 skipped
# vitest 1 file: 30 tests passed

bun run test grows from 959 to 968 cases — the 9 in tests/extensions/plan-mode/result-rendering.test.ts. Those were verified red against the pre-fix code: with renderResult removed, all nine fail with plan_ready must supply renderResult, so they catch the regression rather than document it.

Manual TUI check. Loaded the checkout with pi install, ran /plan, and called plan_ready with a plan exercising headings, ordered and nested lists, a block quote, a table, inline code, bold, italics, links, three fenced code shapes,
and a thematic break. In the TUI: heading markers are consumed, the table renders with box-drawing characters, the quote gets a gutter, links render as label (URL), the thematic break becomes a rule, and fenced code bodies are
indented two spaces.

Expanded-contract verification. Red/blue: with the expanded gate disabled the
bounded-preview case turns red, proving the tests catch a collapsed==expanded regression.
Real TUI with a 390-line plan: collapsed shows the header plus a preview of the plan body and a ... (N more lines) tail, and
Ctrl+O renders the full Markdown.

Two behaviors are pi-tui's design, not regressions:

  • Code fences keep the literal ``` characters — Markdown renders them via theme.codeBlockBorder() (pi-tui/dist/components/markdown.js:384) and colors them rather than drawing a background block.
  • Headings level 3 and deeper keep their ### prefix; only levels 1–2 hide it (markdown.js:350). Level 3+ still receives heading color and bold.

Impact

  • User-visible behavior:plan_ready output now goes through the TUI Markdown renderer instead of the plain-text fallback. Heading markers are consumed, list bullets and code fences are colored, inline code / bold / italic / link
    markers are consumed, tables are drawn with box characters, and fenced code bodies are indented. Nothing else changes.
  • Model-visible context / tools: none. No change to tool name, description, promptSnippet, promptGuidelines, parameters, or the text returned in content — only how the result is drawn.
  • Runtime / lifecycle: none. renderResult is pure presentation; terminate: true semantics and the plan-state commit are untouched.
  • Persisted config / data: none.
  • Compatibility / risk: low. plan_ready remains parent-only (already in CHILD_EXCLUDED_TOOL_NAMES, extensions/shared/child-session.ts:452), so the child-session drift guard is unaffected — its test passes.

Summary by CodeRabbit

  • New Features

    • Plan results now display completed plans with Markdown formatting and consistent visual styling.
    • Empty or missing plan content is shown with a muted fallback message.
    • Expanded and collapsed plan views provide consistent output.
  • Tests

    • Added coverage for headings, code blocks, rich content, large plans, plain text, Unicode, and empty results.

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Must-Fix:extensions/plan-mode/index.ts:455-462 的自定义 renderResult 忽略了 options.expanded,折叠态和展开态都会返回完整 Markdown。Pi 会把 expanded 状态传给自定义 renderer,但不会替它截断 renderer 返回的行;因此接近现有 48 KiB 上限的计划会在默认折叠态整段铺进 transcript,Ctrl+O 也不会产生任何折叠效果。新增测试 tests/extensions/plan-mode/result-rendering.test.ts:242-252 还把 collapsed/expanded 完全相同固定成了预期,这与 PR 中“Pi 已提供 expand/collapse,所以不需要第二层截断”的理由相矛盾。请在 expanded === false 时提供有界摘要/预览,在展开态再渲染完整 Markdown,并补一条长计划测试,证明默认输出行数有界且展开能恢复完整内容。

@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 07858baf-7af6-4723-b714-f6870c0ccccc

📥 Commits

Reviewing files that changed from the base of the PR and between 865f66e and bc0e564.

📒 Files selected for processing (2)
  • extensions/plan-mode/index.ts
  • tests/extensions/plan-mode/result-rendering.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The plan mode extension now renders completed plan_ready results as themed Markdown and shows a muted fallback when plan content is missing. New tests cover Markdown structures, unusual input, and expansion states.

Changes

Plan result rendering

Layer / File(s)Summary
Plan result rendering integration
extensions/plan-mode/index.ts
The plan_ready tool renders completed plans with the shared Markdown theme and displays missing content as muted text.
Rendering behavior validation
tests/extensions/plan-mode/result-rendering.test.ts
Tests cover tool registration, Markdown content, fallback handling, unusual input, code fences, and identical expanded or collapsed output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:⚪ Minimal · up to bc0e5

This change only renders finalized plans as Markdown in the TUI without altering plan content, tools, state, or runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers:tt-a1i

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly and concisely identifies the main change: rendering plan_ready output as Markdown in Plan Mode.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

plan_ready registered no renderResult, so the TUI fell back to its plain-text renderer and the recorded plan appeared as raw Markdown source. Render it with the Markdown component, the same treatment subagent results already get.
Validated with bun run check (format, lint, tsc) and bun run test (959 node:test cases and 30 vitest cases pass) on Linux.
Nine cases pin that the renderer is registered and that a realistic plan survives it: headings, ordered and nested lists, block quotes, tables, inline code and bold, links, three fenced code shapes, a thematic break, a 40K plan, and unicode.
Assertions strip ANSI first, because the Markdown component colors itself from the global theme rather than the theme passed to renderResult.
Verified red against the pre-fix code (all nine fail with "plan_ready must supply renderResult"), so these catch the regression rather than just documenting it.
Collapsed state was identical to expanded: the custom renderResult ignored
the expanded flag, so a plan near the 48KiB cap flooded the transcript and
Ctrl+O produced no visible change. Pi never truncates custom renderer
output; the renderer owns the collapsed/expanded contract (same pattern as
git-read and renderWaitResult).
Collapsed now renders one bounded line with the line count and an expand
hint via keyHint; expanded renders the full Markdown as before. Replaces
the old case that pinned collapsed == expanded with two cases: collapsed
stay bounded and expanded restores the full plan.
@Matt-qwq
Matt-qwqforce-pushed the fix/plan-ready-markdown-render branch from bc0e564 to 80a6e2cCompareAugust 30, 2026 10:41
CodeRabbit docstring coverage scoped to diff-touched functions requires
80%. Adds JSDoc to plan_ready renderResult (index.ts) and the stripAnsi,
loadPlanReady and renderPlan helpers (result-rendering.test.ts).
A bare line count made the collapsed result impossible to scan in a long
session. Collapsed now shows the header (Plan ready · N lines · expand
hint) followed by the first PLAN_PREVIEW_LINES (10) plan lines and a
... (N more lines) tail; the expand hint and tail disappear when the plan
fits the preview, matching bash/fallback and git-read conventions.
Tests: long-plan preview bounded with tail hidden, short plan without
hint, count boundaries at the cap (1/10/11 lines), trailing newline
semantics, narrow-width wrapping of unbroken lines, deterministic output,
and the expanded tail sentinel restore. Red-verified: disabling the
expanded gate fails the five collapsed cases.
@Matt-qwq

Copy link
Copy Markdown
Author

已按 P1 修复完毕:renderResult 现在遵循 expanded 契约——折叠态渲染有界预览(Plan ready · N lines · + 前 10 行内容 + ...(N more lines) 尾注),展开态渲染完整 Markdown。旧测试(断言折叠=展开)已替换为边界用例(行数边界1/10/11、尾换行、窄屏 wrap、幂等、末行 sentinel),红绿验证通过。请重新 review。

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

复审 exact head c39f119373ad1a44402a46418d46144a030559c4。这次已实现 expanded 分支和多行摘要,旧版折叠/展开完全相同的问题已有实质修复。渲染计划为 Markdown 的价值成立,也没有改变模型正文或 Plan Mode 生命周期。

Standards

0 项确认违规;沿用 Pi renderResult,无需新抽象。

Spec

仍有 1 项 P2:预览只限制源文本行,不限制终端换行后的屏幕行。真实 renderer 输入47000字符单行(小于48KiB上限),width=40,折叠态输出1176行,且没有展开提示。默认输出仍可能刷屏;详见行内。

验证:bun run check通过;result-rendering专项15/15通过;上述独立renderer复现通过。完整测试并行尝试遇到setup集成子进程超时,不能记为全套通过;此复现不依赖那个失败。未做安装后真实TUI验收。只需补实际渲染行的裁剪和长单行回归,不需要新增组件框架。

text +=
theme.fg("muted", " · ") + keyHint("app.tools.expand", "to expand");
}
for (const line of lines.slice(0, PLAN_PREVIEW_LINES)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 预览应限制渲染行,而不只是源文本行

这里 slice(0, PLAN_PREVIEW_LINES) 后仍返回会按终端宽度换行的 Text。合法输入 "a".repeat(47000) 只有一行,小于已有48KiB上限;实际 renderResult(...,{expanded:false},theme).render(40) 返回1176行,而且 lines.length === 1 使展开提示不出现。因而这条旧问题只修复了多行输入,长段落/minified内容仍刷屏。请按实际可见行约束折叠预览(并保留展开的完整内容),加一个长单行窄屏测试,断言总行数有界;当前窄屏测试只断言每行宽度。

@tt-a1i

Copy link
Copy Markdown
Collaborator

当前 head c39f119 已复审:旧 expanded 分支已修好,但47000字符单行在40列折叠态仍输出1176行,无展开提示。请按实际渲染行裁剪,详见 #293 (review) 。check和15项专项通过;未宣称全套或真实TUI通过。本轮未改代码或合并。

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Matt-qwq@tt-a1i
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(plan-mode): render plan_ready output as Markdown by Matt-qwq · Pull Request #293 · openpi-dev/openpi · GitHub
Skip to content

fix(plan-mode): render plan_ready output as Markdown - #293

Open
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render
Open

fix(plan-mode): render plan_ready output as Markdown#293
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render

Conversation

@Matt-qwq

@Matt-qwqMatt-qwq commented Aug 29, 2026

Copy link
Copy Markdown

Problem

plan_ready renders the finalized plan as raw Markdown source in the TUI.

extensions/plan-mode/index.ts registers plan_ready with only an execute handler and no renderResult, so the TUI falls back to its default plain-text renderer: headings, lists, and code fences appear as literal #, -, and ```
characters instead of rendered text.

Affects every Plan Mode user — the plan is the one artifact Plan Mode exists to produce, and it is meant to be read, not inspected as source.

No existing issue tracks this. I searched plan_ready, renderResult, and Markdown-rendering across open and closed issues in openpi-dev/openpi; the closest hits (#28, #27, #18, #105, #101, #96, #93, #67, #40) are all about other
subsystems.

Value

Every other extension tool that returns human-readable prose already supplies a renderer — there are 19 renderResult implementations across goal, tasks, git-read, file-search, workflows, subagents, ask-user,
background-terminals, and file-mutation-display. plan_ready is the outlier. Rendering it makes Plan Mode's output consistent with the rest of OpenPI and lets the user read the plan without decoding Markdown syntax.

Approach

Add renderResult to the plan_ready registration, rendering result.details.plan with the Markdown component from @earendil-works/pi-tui using getMarkdownTheme() — the same treatment renderWaitResult() gives subagent results
(extensions/subagents/src/ui/wait-result.ts:59).

Two deliberate choices:

  • Collapsed state shows a bounded preview. Pi passes the expanded flag to custom renderers but never truncates their output — the renderer owns the collapsed/expanded contract (same pattern as git-read and renderWaitResult). Collapsed renders Plan ready · N lines · <to expand> plus the first 10 plan lines and a ... (N more lines) tail; the hint and tail disappear when the plan fits the preview. Only the expanded state renders the full Markdown. Without this, a plan near the 48 KiB cap floods the transcript and Ctrl+O produces no visible change.
  • No re-sanitizing.execute already stores sanitizeTerminalText(params.plan), so details.plan is clean on the way in.

Validation

Ran on WSL2 (Fedora 44), Node v22.23.1, bun 1.3.14:

bun run check # biome format (257 files), biome lint --error-on-warnings, tsc --noEmit
bun run test # node:test 968 cases: 967 pass / 0 fail / 1 skipped
# vitest 1 file: 30 tests passed

bun run test grows from 959 to 968 cases — the 9 in tests/extensions/plan-mode/result-rendering.test.ts. Those were verified red against the pre-fix code: with renderResult removed, all nine fail with plan_ready must supply renderResult, so they catch the regression rather than document it.

Manual TUI check. Loaded the checkout with pi install, ran /plan, and called plan_ready with a plan exercising headings, ordered and nested lists, a block quote, a table, inline code, bold, italics, links, three fenced code shapes,
and a thematic break. In the TUI: heading markers are consumed, the table renders with box-drawing characters, the quote gets a gutter, links render as label (URL), the thematic break becomes a rule, and fenced code bodies are
indented two spaces.

Expanded-contract verification. Red/blue: with the expanded gate disabled the
bounded-preview case turns red, proving the tests catch a collapsed==expanded regression.
Real TUI with a 390-line plan: collapsed shows the header plus a preview of the plan body and a ... (N more lines) tail, and
Ctrl+O renders the full Markdown.

Two behaviors are pi-tui's design, not regressions:

  • Code fences keep the literal ``` characters — Markdown renders them via theme.codeBlockBorder() (pi-tui/dist/components/markdown.js:384) and colors them rather than drawing a background block.
  • Headings level 3 and deeper keep their ### prefix; only levels 1–2 hide it (markdown.js:350). Level 3+ still receives heading color and bold.

Impact

  • User-visible behavior:plan_ready output now goes through the TUI Markdown renderer instead of the plain-text fallback. Heading markers are consumed, list bullets and code fences are colored, inline code / bold / italic / link
    markers are consumed, tables are drawn with box characters, and fenced code bodies are indented. Nothing else changes.
  • Model-visible context / tools: none. No change to tool name, description, promptSnippet, promptGuidelines, parameters, or the text returned in content — only how the result is drawn.
  • Runtime / lifecycle: none. renderResult is pure presentation; terminate: true semantics and the plan-state commit are untouched.
  • Persisted config / data: none.
  • Compatibility / risk: low. plan_ready remains parent-only (already in CHILD_EXCLUDED_TOOL_NAMES, extensions/shared/child-session.ts:452), so the child-session drift guard is unaffected — its test passes.

Summary by CodeRabbit

  • New Features

    • Plan results now display completed plans with Markdown formatting and consistent visual styling.
    • Empty or missing plan content is shown with a muted fallback message.
    • Expanded and collapsed plan views provide consistent output.
  • Tests

    • Added coverage for headings, code blocks, rich content, large plans, plain text, Unicode, and empty results.

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Must-Fix:extensions/plan-mode/index.ts:455-462 的自定义 renderResult 忽略了 options.expanded,折叠态和展开态都会返回完整 Markdown。Pi 会把 expanded 状态传给自定义 renderer,但不会替它截断 renderer 返回的行;因此接近现有 48 KiB 上限的计划会在默认折叠态整段铺进 transcript,Ctrl+O 也不会产生任何折叠效果。新增测试 tests/extensions/plan-mode/result-rendering.test.ts:242-252 还把 collapsed/expanded 完全相同固定成了预期,这与 PR 中“Pi 已提供 expand/collapse,所以不需要第二层截断”的理由相矛盾。请在 expanded === false 时提供有界摘要/预览,在展开态再渲染完整 Markdown,并补一条长计划测试,证明默认输出行数有界且展开能恢复完整内容。

@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 07858baf-7af6-4723-b714-f6870c0ccccc

📥 Commits

Reviewing files that changed from the base of the PR and between 865f66e and bc0e564.

📒 Files selected for processing (2)
  • extensions/plan-mode/index.ts
  • tests/extensions/plan-mode/result-rendering.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The plan mode extension now renders completed plan_ready results as themed Markdown and shows a muted fallback when plan content is missing. New tests cover Markdown structures, unusual input, and expansion states.

Changes

Plan result rendering

Layer / File(s)Summary
Plan result rendering integration
extensions/plan-mode/index.ts
The plan_ready tool renders completed plans with the shared Markdown theme and displays missing content as muted text.
Rendering behavior validation
tests/extensions/plan-mode/result-rendering.test.ts
Tests cover tool registration, Markdown content, fallback handling, unusual input, code fences, and identical expanded or collapsed output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:⚪ Minimal · up to bc0e5

This change only renders finalized plans as Markdown in the TUI without altering plan content, tools, state, or runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers:tt-a1i

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly and concisely identifies the main change: rendering plan_ready output as Markdown in Plan Mode.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

plan_ready registered no renderResult, so the TUI fell back to its plain-text renderer and the recorded plan appeared as raw Markdown source. Render it with the Markdown component, the same treatment subagent results already get.
Validated with bun run check (format, lint, tsc) and bun run test (959 node:test cases and 30 vitest cases pass) on Linux.
Nine cases pin that the renderer is registered and that a realistic plan survives it: headings, ordered and nested lists, block quotes, tables, inline code and bold, links, three fenced code shapes, a thematic break, a 40K plan, and unicode.
Assertions strip ANSI first, because the Markdown component colors itself from the global theme rather than the theme passed to renderResult.
Verified red against the pre-fix code (all nine fail with "plan_ready must supply renderResult"), so these catch the regression rather than just documenting it.
Collapsed state was identical to expanded: the custom renderResult ignored
the expanded flag, so a plan near the 48KiB cap flooded the transcript and
Ctrl+O produced no visible change. Pi never truncates custom renderer
output; the renderer owns the collapsed/expanded contract (same pattern as
git-read and renderWaitResult).
Collapsed now renders one bounded line with the line count and an expand
hint via keyHint; expanded renders the full Markdown as before. Replaces
the old case that pinned collapsed == expanded with two cases: collapsed
stay bounded and expanded restores the full plan.
@Matt-qwq
Matt-qwqforce-pushed the fix/plan-ready-markdown-render branch from bc0e564 to 80a6e2cCompareAugust 30, 2026 10:41
CodeRabbit docstring coverage scoped to diff-touched functions requires
80%. Adds JSDoc to plan_ready renderResult (index.ts) and the stripAnsi,
loadPlanReady and renderPlan helpers (result-rendering.test.ts).
A bare line count made the collapsed result impossible to scan in a long
session. Collapsed now shows the header (Plan ready · N lines · expand
hint) followed by the first PLAN_PREVIEW_LINES (10) plan lines and a
... (N more lines) tail; the expand hint and tail disappear when the plan
fits the preview, matching bash/fallback and git-read conventions.
Tests: long-plan preview bounded with tail hidden, short plan without
hint, count boundaries at the cap (1/10/11 lines), trailing newline
semantics, narrow-width wrapping of unbroken lines, deterministic output,
and the expanded tail sentinel restore. Red-verified: disabling the
expanded gate fails the five collapsed cases.
@Matt-qwq

Copy link
Copy Markdown
Author

已按 P1 修复完毕:renderResult 现在遵循 expanded 契约——折叠态渲染有界预览(Plan ready · N lines · + 前 10 行内容 + ...(N more lines) 尾注),展开态渲染完整 Markdown。旧测试(断言折叠=展开)已替换为边界用例(行数边界1/10/11、尾换行、窄屏 wrap、幂等、末行 sentinel),红绿验证通过。请重新 review。

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

复审 exact head c39f119373ad1a44402a46418d46144a030559c4。这次已实现 expanded 分支和多行摘要,旧版折叠/展开完全相同的问题已有实质修复。渲染计划为 Markdown 的价值成立,也没有改变模型正文或 Plan Mode 生命周期。

Standards

0 项确认违规;沿用 Pi renderResult,无需新抽象。

Spec

仍有 1 项 P2:预览只限制源文本行,不限制终端换行后的屏幕行。真实 renderer 输入47000字符单行(小于48KiB上限),width=40,折叠态输出1176行,且没有展开提示。默认输出仍可能刷屏;详见行内。

验证:bun run check通过;result-rendering专项15/15通过;上述独立renderer复现通过。完整测试并行尝试遇到setup集成子进程超时,不能记为全套通过;此复现不依赖那个失败。未做安装后真实TUI验收。只需补实际渲染行的裁剪和长单行回归,不需要新增组件框架。

text +=
theme.fg("muted", " · ") + keyHint("app.tools.expand", "to expand");
}
for (const line of lines.slice(0, PLAN_PREVIEW_LINES)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 预览应限制渲染行,而不只是源文本行

这里 slice(0, PLAN_PREVIEW_LINES) 后仍返回会按终端宽度换行的 Text。合法输入 "a".repeat(47000) 只有一行,小于已有48KiB上限;实际 renderResult(...,{expanded:false},theme).render(40) 返回1176行,而且 lines.length === 1 使展开提示不出现。因而这条旧问题只修复了多行输入,长段落/minified内容仍刷屏。请按实际可见行约束折叠预览(并保留展开的完整内容),加一个长单行窄屏测试,断言总行数有界;当前窄屏测试只断言每行宽度。

@tt-a1i

Copy link
Copy Markdown
Collaborator

当前 head c39f119 已复审:旧 expanded 分支已修好,但47000字符单行在40列折叠态仍输出1176行,无展开提示。请按实际渲染行裁剪,详见 #293 (review) 。check和15项专项通过;未宣称全套或真实TUI通过。本轮未改代码或合并。

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Matt-qwq@tt-a1i
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); fix(plan-mode): render plan_ready output as Markdown by Matt-qwq · Pull Request #293 · openpi-dev/openpi · GitHub
Skip to content

fix(plan-mode): render plan_ready output as Markdown - #293

Open
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render
Open

fix(plan-mode): render plan_ready output as Markdown#293
Matt-qwq wants to merge 5 commits into
openpi-dev:mainfrom
Matt-qwq:fix/plan-ready-markdown-render

Conversation

@Matt-qwq

@Matt-qwqMatt-qwq commented Aug 29, 2026

Copy link
Copy Markdown

Problem

plan_ready renders the finalized plan as raw Markdown source in the TUI.

extensions/plan-mode/index.ts registers plan_ready with only an execute handler and no renderResult, so the TUI falls back to its default plain-text renderer: headings, lists, and code fences appear as literal #, -, and ```
characters instead of rendered text.

Affects every Plan Mode user — the plan is the one artifact Plan Mode exists to produce, and it is meant to be read, not inspected as source.

No existing issue tracks this. I searched plan_ready, renderResult, and Markdown-rendering across open and closed issues in openpi-dev/openpi; the closest hits (#28, #27, #18, #105, #101, #96, #93, #67, #40) are all about other
subsystems.

Value

Every other extension tool that returns human-readable prose already supplies a renderer — there are 19 renderResult implementations across goal, tasks, git-read, file-search, workflows, subagents, ask-user,
background-terminals, and file-mutation-display. plan_ready is the outlier. Rendering it makes Plan Mode's output consistent with the rest of OpenPI and lets the user read the plan without decoding Markdown syntax.

Approach

Add renderResult to the plan_ready registration, rendering result.details.plan with the Markdown component from @earendil-works/pi-tui using getMarkdownTheme() — the same treatment renderWaitResult() gives subagent results
(extensions/subagents/src/ui/wait-result.ts:59).

Two deliberate choices:

  • Collapsed state shows a bounded preview. Pi passes the expanded flag to custom renderers but never truncates their output — the renderer owns the collapsed/expanded contract (same pattern as git-read and renderWaitResult). Collapsed renders Plan ready · N lines · <to expand> plus the first 10 plan lines and a ... (N more lines) tail; the hint and tail disappear when the plan fits the preview. Only the expanded state renders the full Markdown. Without this, a plan near the 48 KiB cap floods the transcript and Ctrl+O produces no visible change.
  • No re-sanitizing.execute already stores sanitizeTerminalText(params.plan), so details.plan is clean on the way in.

Validation

Ran on WSL2 (Fedora 44), Node v22.23.1, bun 1.3.14:

bun run check # biome format (257 files), biome lint --error-on-warnings, tsc --noEmit
bun run test # node:test 968 cases: 967 pass / 0 fail / 1 skipped
# vitest 1 file: 30 tests passed

bun run test grows from 959 to 968 cases — the 9 in tests/extensions/plan-mode/result-rendering.test.ts. Those were verified red against the pre-fix code: with renderResult removed, all nine fail with plan_ready must supply renderResult, so they catch the regression rather than document it.

Manual TUI check. Loaded the checkout with pi install, ran /plan, and called plan_ready with a plan exercising headings, ordered and nested lists, a block quote, a table, inline code, bold, italics, links, three fenced code shapes,
and a thematic break. In the TUI: heading markers are consumed, the table renders with box-drawing characters, the quote gets a gutter, links render as label (URL), the thematic break becomes a rule, and fenced code bodies are
indented two spaces.

Expanded-contract verification. Red/blue: with the expanded gate disabled the
bounded-preview case turns red, proving the tests catch a collapsed==expanded regression.
Real TUI with a 390-line plan: collapsed shows the header plus a preview of the plan body and a ... (N more lines) tail, and
Ctrl+O renders the full Markdown.

Two behaviors are pi-tui's design, not regressions:

  • Code fences keep the literal ``` characters — Markdown renders them via theme.codeBlockBorder() (pi-tui/dist/components/markdown.js:384) and colors them rather than drawing a background block.
  • Headings level 3 and deeper keep their ### prefix; only levels 1–2 hide it (markdown.js:350). Level 3+ still receives heading color and bold.

Impact

  • User-visible behavior:plan_ready output now goes through the TUI Markdown renderer instead of the plain-text fallback. Heading markers are consumed, list bullets and code fences are colored, inline code / bold / italic / link
    markers are consumed, tables are drawn with box characters, and fenced code bodies are indented. Nothing else changes.
  • Model-visible context / tools: none. No change to tool name, description, promptSnippet, promptGuidelines, parameters, or the text returned in content — only how the result is drawn.
  • Runtime / lifecycle: none. renderResult is pure presentation; terminate: true semantics and the plan-state commit are untouched.
  • Persisted config / data: none.
  • Compatibility / risk: low. plan_ready remains parent-only (already in CHILD_EXCLUDED_TOOL_NAMES, extensions/shared/child-session.ts:452), so the child-session drift guard is unaffected — its test passes.

Summary by CodeRabbit

  • New Features

    • Plan results now display completed plans with Markdown formatting and consistent visual styling.
    • Empty or missing plan content is shown with a muted fallback message.
    • Expanded and collapsed plan views provide consistent output.
  • Tests

    • Added coverage for headings, code blocks, rich content, large plans, plain text, Unicode, and empty results.

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Must-Fix:extensions/plan-mode/index.ts:455-462 的自定义 renderResult 忽略了 options.expanded,折叠态和展开态都会返回完整 Markdown。Pi 会把 expanded 状态传给自定义 renderer,但不会替它截断 renderer 返回的行;因此接近现有 48 KiB 上限的计划会在默认折叠态整段铺进 transcript,Ctrl+O 也不会产生任何折叠效果。新增测试 tests/extensions/plan-mode/result-rendering.test.ts:242-252 还把 collapsed/expanded 完全相同固定成了预期,这与 PR 中“Pi 已提供 expand/collapse,所以不需要第二层截断”的理由相矛盾。请在 expanded === false 时提供有界摘要/预览,在展开态再渲染完整 Markdown,并补一条长计划测试,证明默认输出行数有界且展开能恢复完整内容。

@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 07858baf-7af6-4723-b714-f6870c0ccccc

📥 Commits

Reviewing files that changed from the base of the PR and between 865f66e and bc0e564.

📒 Files selected for processing (2)
  • extensions/plan-mode/index.ts
  • tests/extensions/plan-mode/result-rendering.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The plan mode extension now renders completed plan_ready results as themed Markdown and shows a muted fallback when plan content is missing. New tests cover Markdown structures, unusual input, and expansion states.

Changes

Plan result rendering

Layer / File(s)Summary
Plan result rendering integration
extensions/plan-mode/index.ts
The plan_ready tool renders completed plans with the shared Markdown theme and displays missing content as muted text.
Rendering behavior validation
tests/extensions/plan-mode/result-rendering.test.ts
Tests cover tool registration, Markdown content, fallback handling, unusual input, code fences, and identical expanded or collapsed output.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:⚪ Minimal · up to bc0e5

This change only renders finalized plans as Markdown in the TUI without altering plan content, tools, state, or runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers:tt-a1i

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly and concisely identifies the main change: rendering plan_ready output as Markdown in Plan Mode.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

plan_ready registered no renderResult, so the TUI fell back to its plain-text renderer and the recorded plan appeared as raw Markdown source. Render it with the Markdown component, the same treatment subagent results already get.
Validated with bun run check (format, lint, tsc) and bun run test (959 node:test cases and 30 vitest cases pass) on Linux.
Nine cases pin that the renderer is registered and that a realistic plan survives it: headings, ordered and nested lists, block quotes, tables, inline code and bold, links, three fenced code shapes, a thematic break, a 40K plan, and unicode.
Assertions strip ANSI first, because the Markdown component colors itself from the global theme rather than the theme passed to renderResult.
Verified red against the pre-fix code (all nine fail with "plan_ready must supply renderResult"), so these catch the regression rather than just documenting it.
Collapsed state was identical to expanded: the custom renderResult ignored
the expanded flag, so a plan near the 48KiB cap flooded the transcript and
Ctrl+O produced no visible change. Pi never truncates custom renderer
output; the renderer owns the collapsed/expanded contract (same pattern as
git-read and renderWaitResult).
Collapsed now renders one bounded line with the line count and an expand
hint via keyHint; expanded renders the full Markdown as before. Replaces
the old case that pinned collapsed == expanded with two cases: collapsed
stay bounded and expanded restores the full plan.
@Matt-qwq
Matt-qwqforce-pushed the fix/plan-ready-markdown-render branch from bc0e564 to 80a6e2cCompareAugust 30, 2026 10:41
CodeRabbit docstring coverage scoped to diff-touched functions requires
80%. Adds JSDoc to plan_ready renderResult (index.ts) and the stripAnsi,
loadPlanReady and renderPlan helpers (result-rendering.test.ts).
A bare line count made the collapsed result impossible to scan in a long
session. Collapsed now shows the header (Plan ready · N lines · expand
hint) followed by the first PLAN_PREVIEW_LINES (10) plan lines and a
... (N more lines) tail; the expand hint and tail disappear when the plan
fits the preview, matching bash/fallback and git-read conventions.
Tests: long-plan preview bounded with tail hidden, short plan without
hint, count boundaries at the cap (1/10/11 lines), trailing newline
semantics, narrow-width wrapping of unbroken lines, deterministic output,
and the expanded tail sentinel restore. Red-verified: disabling the
expanded gate fails the five collapsed cases.
@Matt-qwq

Copy link
Copy Markdown
Author

已按 P1 修复完毕:renderResult 现在遵循 expanded 契约——折叠态渲染有界预览(Plan ready · N lines · + 前 10 行内容 + ...(N more lines) 尾注),展开态渲染完整 Markdown。旧测试(断言折叠=展开)已替换为边界用例(行数边界1/10/11、尾换行、窄屏 wrap、幂等、末行 sentinel),红绿验证通过。请重新 review。

@tt-a1itt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

复审 exact head c39f119373ad1a44402a46418d46144a030559c4。这次已实现 expanded 分支和多行摘要,旧版折叠/展开完全相同的问题已有实质修复。渲染计划为 Markdown 的价值成立,也没有改变模型正文或 Plan Mode 生命周期。

Standards

0 项确认违规;沿用 Pi renderResult,无需新抽象。

Spec

仍有 1 项 P2:预览只限制源文本行,不限制终端换行后的屏幕行。真实 renderer 输入47000字符单行(小于48KiB上限),width=40,折叠态输出1176行,且没有展开提示。默认输出仍可能刷屏;详见行内。

验证:bun run check通过;result-rendering专项15/15通过;上述独立renderer复现通过。完整测试并行尝试遇到setup集成子进程超时,不能记为全套通过;此复现不依赖那个失败。未做安装后真实TUI验收。只需补实际渲染行的裁剪和长单行回归,不需要新增组件框架。

text +=
theme.fg("muted", " · ") + keyHint("app.tools.expand", "to expand");
}
for (const line of lines.slice(0, PLAN_PREVIEW_LINES)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 预览应限制渲染行,而不只是源文本行

这里 slice(0, PLAN_PREVIEW_LINES) 后仍返回会按终端宽度换行的 Text。合法输入 "a".repeat(47000) 只有一行,小于已有48KiB上限;实际 renderResult(...,{expanded:false},theme).render(40) 返回1176行,而且 lines.length === 1 使展开提示不出现。因而这条旧问题只修复了多行输入,长段落/minified内容仍刷屏。请按实际可见行约束折叠预览(并保留展开的完整内容),加一个长单行窄屏测试,断言总行数有界;当前窄屏测试只断言每行宽度。

@tt-a1i

Copy link
Copy Markdown
Collaborator

当前 head c39f119 已复审:旧 expanded 分支已修好,但47000字符单行在40列折叠态仍输出1176行,无展开提示。请按实际渲染行裁剪,详见 #293 (review) 。check和15项专项通过;未宣称全套或真实TUI通过。本轮未改代码或合并。

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Matt-qwq@tt-a1i