Uh oh!
There was an error while loading. Please reload this page.
fix(engine): serialize validator and dialog-evaluator prompts with ensure_ascii=False - #359
Conversation
e1f3568 to
25384feCompare…sure_ascii=False The validator grader and the dialog-trigger evaluator serialized the agent output with json.dumps() using the default ensure_ascii=True before applying their fixed character budgets (8000 and 4000 chars). Every Cyrillic/CJK code point inflated to six \uXXXX characters (emoji to twelve), so a non-English output reached the grader with up to ~6x less source material than an equivalent ASCII output, and the cut could land inside an escape sequence, leaving malformed JSON in the prompt. Both now serialize with ensure_ascii=False, so the budget is measured in real characters for every language and truncation cannot split an escape sequence. Regression tests cover both prompts with CJK output that fits the budget unescaped but would be truncated escaped. Fixesmicrosoft#356
25384fe to
8ac5834Compare
Jason Robert (jrob5756)
left a comment
There was a problem hiding this comment.
Nice fix, and the tests actually prove it, I reverted ensure_ascii=False locally and both new tests fail as expected!
One gap worth a follow-up: the same escaping bug still lives in src/conductor/gates/dialog.py (lines 222, 388, 612), all doing json.dumps(agent_output, indent=2, default=str) with no ensure_ascii=False. Lines 222 and 388 feed the same output into the dialog-mode LLM system prompt, so non-ASCII agent output reaches the model escaped. Line 612 is worse in a different way: it renders straight into the console panel a person is looking at during a dialog session, so someone running a workflow on Cyrillic or CJK output would see literal \u4f60\u597d instead of the real text. None of these have a fixed truncation budget, so it's not the exact mid-escape-cut bug from #356, but it's the same root cause and the same one-line fix.
Lower priority but same pattern: src/conductor/engine/workflow.py lines 2435, 2505, 2630 build [:500] log/console previews the same escaped way, so non-ASCII previews can come out garbled. Cosmetic, but worth folding in while you're touching this.
Neither is blocking for this PR's stated scope, just flagging so #356 doesn't need a part two.
…ensure_ascii=False Follow-up to the validator fix addressing the same root cause flagged in review: gates/dialog.py serialized the agent output with the default ensure_ascii=True in three places — the CLI and web dialog-mode LLM system prompts, and the "Agent Output" console panel a person reads during a CLI dialog session — so non-ASCII output reached the model (and the user) as \uXXXX escapes. The three [:500] output previews in engine/workflow.py (interrupt handler preview and the agent_paused partial-content previews) had the same escaping, so non-ASCII previews could come out garbled. All six call sites now serialize with ensure_ascii=False. Regression tests cover the CLI and web dialog system prompts, the console panel renderable, and the interrupt preview with Cyrillic/CJK output; each fails if the fix is reverted. Refs microsoft#356
hertznsk
left a comment
There was a problem hiding this comment.
Thanks for the thorough check — great catch on the remaining call sites. I've folded them into this PR in c910c2b rather than leaving them for a follow-up:
gates/dialog.py(all three): the CLI + web dialog system prompts and the console "Agent Output" panel now serialize withensure_ascii=False, so the dialog-mode LLM and the person at the terminal both see real non-ASCII text.engine/workflow.py(all three): the[:500]previews (interrupt handler preview and theagent_pausedpartial-content previews) now serialize the same way.
Regression tests added for each surface — CLI dialog system prompt, web dialog system prompt, the console panel renderable, and the interrupt preview — with mixed Cyrillic/CJK output, asserting the literal text is present and no \\u4f60-style escapes leak. I also verified each new test fails when the fix is reverted, same as you did for the original pair.
make check (ruff + ty) and the engine/gates/executor test suites are green (1247 passed, 1 skipped).
Jason Robert (jrob5756)
left a comment
There was a problem hiding this comment.
LGTM. Approved!
Summary
Fixes#356.
OutputValidator.validate()serialized the primary agent output withjson.dumps(primary_output, indent=2, default=str)(defaultensure_ascii=True) and only then applied the fixed_OUTPUT_LIMIT = 8000character budget. Non-ASCII text inflates into\uXXXXescapes before truncation — 6 chars per Cyrillic/CJK code point, 12 per emoji — so non-English outputs reached the grader with up to ~6x less source material than an equivalent ASCII output, and the cut could land inside an escape sequence, leaving malformed JSON in the embeddedAGENT OUTPUTsection.Changes
src/conductor/engine/validator.py— serialize withensure_ascii=Falseso the 8000-char budget is measured in real characters for every language and truncation cannot split an escape sequence.src/conductor/engine/dialog_evaluator.py— a codebase scan found the identical serialize-then-truncate pattern feeding the dialog-trigger evaluator prompt (4000-char budget); fixed the same way.test_validator.py,test_dialog_evaluator.py) — CJK output that fits the budget unescaped (~2 KB / ~1 KB) but would be truncated escaped (~12 KB / ~6 KB) must reach the prompt unescaped, with no\uXXXXescapes and no truncation marker.Other
json.dumpscall sites were reviewed and intentionally left alone: they are system serialization (checkpoints, NDJSON event logs, CLI argv, cache files) or prompt content without fixed-budget truncation, where ASCII escaping has no fairness or correctness impact.Test plan
pytest tests/test_engine/test_validator.py tests/test_engine/test_validator_integration.py tests/test_engine/test_dialog_evaluator.py— 61/61 passruff check/ruff format --check/ty check— clean on all touched files