Skip to content

fix(cli): show non-ASCII workflow inputs literally in verbose output - #391

Merged
Jason Robert (jrob5756) merged 1 commit into
microsoft:mainfrom
hertznsk:fix/371-verbose-rendering
Aug 11, 2026
Merged

fix(cli): show non-ASCII workflow inputs literally in verbose output#391
Jason Robert (jrob5756) merged 1 commit into
microsoft:mainfrom
hertznsk:fix/371-verbose-rendering

Conversation

@hertznsk

@hertznskhertznsk commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Serialise the verbose "Workflow Inputs" panel with ensure_ascii=False so non-ASCII input values are shown as the text the user typed rather than \uXXXX escapes.

Why

json.dumps(inputs, indent=2) used the default ensure_ascii=True, so running a workflow with a Cyrillic, CJK or emoji input rendered the panel as:

{
"question": "\u043f\u043b\u0430\u043d \u4f60\u597d"
}

Every other JSON display path in the repo already passes ensure_ascii=False (issue #356, PR #359) with a comment explaining why: gates/dialog.py, engine/workflow.py, engine/validator.py, engine/dialog_evaluator.py. This call site was the last one that did not.

Scope change

This branch originally fixed Rich parsing bracketed prompt text as console markup (#371). That problem was fixed independently on main by #387, which reached the same Panel(Text(content), ...) console fix and additionally set markup=False on the file-log console.

Measured on current main: the #371 reproduction (175 KB of link-heavy prompt) renders in 388 ms and 2.7 MB, down from 63 s and 993 MB, with the file log preserving content verbatim. The rest of this branch is therefore rebased away as redundant, and the duplicate test is dropped since tests/test_cli/test_markup_safety.py from #387 already covers both sinks more thoroughly.

Validation

  • New regression test test_workflow_inputs_section_serializes_non_ascii_unescaped, which fails without the change on assert 'план 你好' in ...
  • Full suite: 5509 passed, 37 skipped
  • ruff check, ruff format --check and ty check all clean

@jrob5756Jason Robert (jrob5756) 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.

Nice find here. I reproduced #371 on main (175 KB of link-heavy prompt: 63s, 993 MB) and this branch renders the same input in about 0.3s. The file-log half is a real improvement the issue never even asked for, since Text(content) makes the --log-file artifact a faithful copy for the first time.

My one substantive worry is Markdown on the console. Rich's Markdown renderer rewrites content rather than escaping it, and some of what it discards matters here. A prompt wrapped in XML-ish tags with no blank lines renders as an empty panel. That is not hypothetical: skills/loader.py wraps injected skills in <skills>/<skill> tags and executor/agent.py prepends that before this logs it, so on claude and hermes the console shows a prompt with every injected skill missing. Link URLs vanish too, which is the exact content class from the issue's own reproduction.

rich.text.Text closes #371 just as completely, measured about 5x faster and 3x lighter, and it keeps the panel honest. It also makes the ```json fence unnecessary. That was option 1 in the issue, and I think it is the better trade. Details inline.

Comment on lines +382 to +384
_verbose_console.print(
Panel(Markdown(content), title=f"[cyan]{title}[/cyan]", border_style="dim")
)

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.

Rich's Markdown renderer does not just neutralise the brackets, it rewrites the content. Confirmed to disappear from the panel: HTML/XML blocks, link URLs ([text](url) renders as bare text), leading indentation, and _underscores_. A tag-structured prompt with no blank lines renders as a completely empty box.

That last one bites in practice. skills/loader.py:147 wraps injected skills in <skills>/<skill name=...>, and executor/agent.py prepends that to the prompt before this line logs it, so on claude and hermes the console silently shows the prompt minus every injected skill.

Text avoids markup parsing just as completely (0.22s / 2.9 MB against 1.15s / 9.0 MB for a 175 KB prompt) and shows what was actually sent. If you take this, also drop the now-unused Markdown import on line 370 and the fence on line 1713.

If you would rather keep Markdown, then the content: entry in the docstring needs to say so, because content has quietly become Markdown source and executor/agent.py passes arbitrary prompt text straight into it.

Suggested change
_verbose_console.print(
Panel(Markdown(content), title=f"[cyan]{title}[/cyan]", border_style="dim")
)
_verbose_console.print(
Panel(Text(content), title=f"[cyan]{title}[/cyan]", border_style="dim")
)

Panel(Markdown(content), title=f"[cyan]{title}[/cyan]", border_style="dim")
)

# File always gets full untruncated content

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.

Minor archaeology, worth fixing while you are in here. Commit 87476b7 deleted the truncate parameter and added this comment in the same change, so the "untruncated" contrast has never had anything to contrast against. Nothing in this function truncates the console.

What is true now is a fidelity difference, and it is exactly what the Text on the next line buys you.

Suggested change
# File always gets full untruncated content
# File sink is written even when the console sink is gated off, and records
# content verbatim: Text parses no markup, so the log is a faithful copy.

Comment threadsrc/conductor/cli/run.py Outdated

if inputs:
verbose_log_section("Workflow Inputs", json.dumps(inputs, indent=2))
verbose_log_section("Workflow Inputs", f"```json\n{json.dumps(inputs, indent=2)}\n```")

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.

This fence is load-bearing and nothing says so. Drop it and Markdown reflows the payload onto one line, losing indent=2 entirely.

Two side effects worth knowing. The backticks reach the file sink as well, so the plain-text log now carries markdown syntax that exists only to satisfy the console. And nothing covers it: I reverted this hunk on its own and all 4968 tests still passed.

Moving the console to Text removes the problem rather than working around it. While you are on this line, it is also the last JSON display path in the repo still on the default ensure_ascii, so Cyrillic and CJK inputs render as \uXXXX. Compare gates/dialog.py:616 and engine/workflow.py:3113, which both carry a comment explaining the flag.

Suggested change
verbose_log_section("Workflow Inputs", f"```json\n{json.dumps(inputs, indent=2)}\n```")
verbose_log_section("Workflow Inputs", json.dumps(inputs, indent=2, ensure_ascii=False))

Comment threadtests/test_cli/test_logging.py Outdated
Comment on lines +485 to +515
from unittest.mock import MagicMock

from rich.markdown import Markdown
from rich.panel import Panel
from rich.text import Text

from conductor.cli import run as run_module

console = MagicMock()
file_console = MagicMock()
token_verbose = verbose_mode.set(True)
token_full = full_mode.set(True)
try:
with (
patch("conductor.cli.run._verbose_console", console),
patch("conductor.cli.run._file_console", file_console),
):
# Requirement: bracket-heavy agent prompts render through the Markdown parser.
content = "**Release** [[`abc123`](https://example.com/commit/abc123)]"
run_module.verbose_log_section("Prompt", content)
finally:
full_mode.reset(token_full)
verbose_mode.reset(token_verbose)

console_panel = console.print.call_args.args[0]
file_panel = file_console.print.call_args.args[0]
assert isinstance(console_panel, Panel)
assert isinstance(console_panel.renderable, Markdown)
assert isinstance(file_panel, Panel)
assert isinstance(file_panel.renderable, Text)
assert file_panel.renderable.plain == content

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.

This asserts the renderer class rather than the behaviour, and that has a real cost. I swapped the console to Panel(Text(content)), which fixes #371 identically and loses no content, and this test fails on it. It rejects a correct fix.

It also never touches the brackets. Re-run it with content = "hello" and it still passes, so the witness string is currently decorative.

Below is a version I validated three ways: passes on this branch, passes with the Text alternative, and fails against pre-fix code on assert "[bold]literal[/bold]" in rendered. It uses the real-Console-into-StringIO pattern the neighbouring tests already use, and keeps your .plain == content assertion, which is the one genuinely behavioural check in the original.

Suggested change
fromunittest.mockimportMagicMock
fromrich.markdownimportMarkdown
fromrich.panelimportPanel
fromrich.textimportText
fromconductor.cliimportrunasrun_module
console=MagicMock()
file_console=MagicMock()
token_verbose=verbose_mode.set(True)
token_full=full_mode.set(True)
try:
with (
patch("conductor.cli.run._verbose_console", console),
patch("conductor.cli.run._file_console", file_console),
):
# Requirement: bracket-heavy agent prompts render through the Markdown parser.
content="**Release** [[`abc123`](https://example.com/commit/abc123)]"
run_module.verbose_log_section("Prompt", content)
finally:
full_mode.reset(token_full)
verbose_mode.reset(token_verbose)
console_panel=console.print.call_args.args[0]
file_panel=file_console.print.call_args.args[0]
assertisinstance(console_panel, Panel)
assertisinstance(console_panel.renderable, Markdown)
assertisinstance(file_panel, Panel)
assertisinstance(file_panel.renderable, Text)
assertfile_panel.renderable.plain==content
importre
fromioimportStringIO
fromunittest.mockimportMagicMock
fromrich.consoleimportConsole
fromrich.panelimportPanel
fromrich.textimportText
fromconductor.cliimportrunasrun_module
output=StringIO()
file_console=MagicMock()
token_verbose=verbose_mode.set(True)
token_full=full_mode.set(True)
try:
with (
patch(
"conductor.cli.run._verbose_console",
Console(file=output, force_terminal=True, no_color=True, width=200),
),
patch("conductor.cli.run._file_console", file_console),
):
# Issue #371 regression witness: keep the brackets. Drop them and every
# assertion below still passes on any string at all.
content="[bold]literal[/bold] [#63111] [/] tail"
run_module.verbose_log_section("Prompt", content)
finally:
full_mode.reset(token_full)
verbose_mode.reset(token_verbose)
rendered=re.sub(r"\x1b\[[0-9;]*m", "", output.getvalue())
assert"[bold]literal[/bold]"inrendered
assert"[#63111]"inrendered
assert"tail"inrendered
file_panel=file_console.print.call_args.args[0]
assertisinstance(file_panel, Panel)
assertisinstance(file_panel.renderable, Text)
assertfile_panel.renderable.plain==content

@hertznskhertznsk changed the title fix(cli): render verbose sections as Markdownfix(cli): preserve verbose section content literallyAug 10, 2026
@hertznsk

Copy link
Copy Markdown
ContributorAuthor

Thanks, Jason Robert (@jrob5756) — I switched the verbose console path to Text(content) so it fixes #371 without rewriting the diagnostic payload. The updated regression test now exercises the rendered behavior and covers Rich markup literals, XML-ish skill wrappers, and Markdown link source; the file sink still records the literal content.

I also removed the workflow-input Markdown fence, added ensure_ascii=False, and updated the stale file-sink comment. Markdown formatting itself was not a requirement for me: I chose it as an alternative to Rich markup because I assumed preserving the visual formatting might matter, but preserving exactly what was sent is the better contract for verbose diagnostics.

@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@a58c88f). Learn more about missing BASE report.

Additional details and impacted files
@@ Coverage Diff @@## main #391 +/- ##
=======================================
Coverage ? 91.32% =======================================
Files ? 108 Lines ? 17507 Branches ? 0 =======================================
Hits ? 15989 Misses ? 1518 Partials ? 0 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The "Workflow Inputs" verbose panel serialised inputs with the default
ensure_ascii=True, so a Cyrillic, CJK or emoji input value was displayed
as \uXXXX escapes rather than the text the user typed. Every other JSON
display path in the repo already passes ensure_ascii=False (issue microsoft#356);
this was the last one that did not.
This branch previously also carried a fix for Rich parsing bracketed
prompt text as console markup (microsoft#371). That landed independently on main
in microsoft#387, which arrived at the same Panel(Text(content), ...) console fix
and additionally set markup=False on the file-log console. The rest of
this branch is therefore rebased away as redundant, leaving only the
encoding change and its regression test.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jrob5756Jason Robert (jrob5756) changed the title fix(cli): preserve verbose section content literallyfix(cli): show non-ASCII workflow inputs literally in verbose outputAug 11, 2026
@jrob5756

Copy link
Copy Markdown
Collaborator

Heads up hertznsk, I rebased this branch and force-pushed, so please git fetch and reset before doing any more work on it.

Your v2 was correct. It got overtaken. While it was in review, #387 merged and fixed the same root cause in the same function, landing on main about three hours ago. It reached the identical Panel(Text(content), ...) console fix you did, arrived at independently, and additionally set markup=False on the file-log console, which makes the Text() on the file sink redundant. That put the branch in conflict and left most of it duplicating code already on main.

I checked what was still unique after #387 and it was one thing: ensure_ascii=False on the workflow-inputs json.dumps. So the branch is now that change plus a regression test, retitled to match. I dropped the added test in test_logging.py because tests/test_cli/test_markup_safety.py from #387 already covers both sinks, including a case yours did not: opening tags like [bold], which Rich swallows without raising, so a crash-only test cannot tell a real fix from one that silently drops text.

Worth recording since the issue is still open: #371 is fixed on main. I measured the 175 KB reproduction at 388 ms and 2.7 MB, against 63 s and 993 MB before, and the file log keeps content verbatim.

Thanks for the work here, and for the thorough follow-up on the review comments. The diagnosis in #371 was accurate and the benchmarks in it were genuinely useful for verifying #387 too.

@jrob5756Jason Robert (jrob5756) 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.

LGTM. Approved!

@jrob5756
Jason Robert (jrob5756) merged commit abe55b5 into microsoft:mainAug 11, 2026
10 checks passed
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.

3 participants

@hertznsk@codecov-commenter@jrob5756