Skip to content

feat(ipynb): render code cell outputs (streams, errors, text results) - #2534

Open
Manohar Paturi (ManoharPaturi) wants to merge 2 commits into
microsoft:mainfrom
ManoharPaturi:feat/ipynb-outputs
Open

Manohar Paturi (ManoharPaturi) wants to merge 2 commits into
microsoft:mainfrom
ManoharPaturi:feat/ipynb-outputs

Conversation

@ManoharPaturi

Copy link
Copy Markdown

Fixes #2285.

Code cell outputs were dropped entirely; only the sources converted. This renders the text-bearing ones after each cell:

  • stream outputs (stdout/stderr) as fenced blocks
  • error outputs with the recorded traceback, falling back to ename: evalue
  • execute_result / display_data text/plain payloads

Binary outputs (images, HTML) are intentionally not handled. Cells without outputs are byte-for-byte unchanged (the empty-render join is skipped), so existing conversions do not change shape.

3 new tests (stream+error, text result, no-output cell unchanged); the first two fail on main. Notebook-related suites green (9 passed).

Notebooks carry their recorded results in cell outputs, but the
converter dropped them entirely: only sources survived. Render the
text-bearing outputs after each code cell - stdout/stderr streams and
plain-text results as fenced blocks, error outputs with their traceback
(or ename/evalue when no traceback was recorded). Image/HTML outputs are
left out.

Fixes microsoft#2285

Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 18, 2026 12:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Output is put inside a fixed triple-backtick fence without checking its contents. A cell that prints a line containing three backticks will close the fence early and corrupt the rest of the Markdown. Could this pick a fence longer than any backtick run in the output?

…tent

Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
@ManoharPaturi

Copy link
Copy Markdown
Author

good catch, that would corrupt the document badly. fixed with a small helper that scans the content for the longest backtick run and uses a fence one tick longer (min 3, so normal outputs are unchanged). applied to code sources, streams, errors, text results and raw cells. added a regression test that prints a triple-backtick line and asserts the output fence grows to four ticks while a backtick-free source fence stays at three.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rechecked b24f82b. The fence is now chosen one backtick longer than the longest run in the cell/output content, so embedded triple-backtick (or longer) runs cannot terminate the block early. The regression covers the exact case I raised, and the focused notebook-output suite passes 4/4 locally. My concern is resolved.

@ManoharPaturi

Copy link
Copy Markdown
Author

thanks again for the careful review, approving this and #2529 means a lot. the fence-length idea was exactly the right call, it's the same approach python's repr uses for quote selection so it felt natural here.

Sign up for free to 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.

3 participants