Repository navigation
fix(workflows): keep non-ASCII text readable in run artifacts - #4877
Open
NishilRathod wants to merge 1 commit into
Open
NishilRathod wants to merge 1 commit into
NishilRathod wants to merge 1 commit into
Conversation
The run records under .specify/workflows/runs/<run_id>/ are meant to be human-auditable, but every writer used the serializer's ASCII-only default, so non-English text in the definition snapshot (workflow.yml), state.json, inputs.json and log.jsonl came out as \uXXXX escapes. Write them as authored: allow_unicode=True for the YAML snapshot and ensure_ascii=False for the JSON/JSONL writers, as github#4148 and github#4773 did for overlay files and merged settings. The JSON writers also use errors="backslashreplace", so a lone surrogate (an undecodable byte in a CLI argument) is still written as its JSON \u escape and loads back unchanged instead of failing the save. Fixes github#4875 Assisted-by: Claude Code (model: Claude Opus 5.5, autonomous) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused serialization changes are consistent with existing patterns and have appropriate regression coverage.
0 open findings
What changed in this PR
Makes workflow run artifacts human-readable for non-ASCII content while safely preserving lone surrogates.
Changes:
- Enables Unicode output for YAML and JSON artifacts.
- Adds non-ASCII readability and surrogate round-trip regression tests.
- Baseline results rely on the provided fail-before/pass-after evidence.
| File | Description |
|---|---|
src/specify_cli/workflows/engine.py |
Writes readable Unicode with safe surrogate fallback. |
tests/test_workflows.py |
Covers all affected artifacts and edge cases. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #4875.
Workflow run records under
.specify/workflows/runs/<run_id>/are meant to be human-auditable, but every writer used its serializer's ASCII-only default. Any non-English text in the run came out as\uXXXXescapes. This covers the definition snapshot (workflow.yml),state.json,inputs.jsonandlog.jsonl.This change writes the text as authored, using the same approach as #4148 (overlay YAML) and #4773 (merged JSON settings). All edits are in
src/specify_cli/workflows/engine.py:workflow.ymlWorkflowEngine.executeyaml.safe_dump(..., allow_unicode=True)state.json,inputs.jsonRunState._atomic_write_jsonjson.dump(..., ensure_ascii=False), temp file opened witherrors="backslashreplace"log.jsonlRunState.append_logjson.dumps(..., ensure_ascii=False), file opened witherrors="backslashreplace"Why
errors="backslashreplace": withensure_ascii=False, a lone surrogate can't be encoded as UTF-8. One can arrive from an undecodable byte in a CLI argument viasurrogateescape, and it would makesave()raise. backslashreplace writes it as\udc80, which is itself a valid JSON escape, so it loads back as the same string. The YAML snapshot doesn't need this, because PyYAML already escapes non-printable characters, surrogates included, underallow_unicode=True. Nothing insrc/reads these files back exceptRunState.load, which already usesencoding="utf-8".Testing
uv run specify --helpuv sync && uv run pytestNew tests in
tests/test_workflows.py::TestRunState:test_run_artifacts_keep_non_ascii_text_readable: CJK, Spanish and em-dash text in inputs, step results and a log entry must appear verbatim instate.json,inputs.jsonandlog.jsonl, and round-trip throughRunState.load.test_workflow_snapshot_keeps_non_ascii_text_readable:engine.execute()on a workflow with a CJKnamemust write it verbatim in the run'sworkflow.yml.test_run_artifacts_round_trip_a_lone_surrogate: an input containing"\udc80"must save and load back unchanged. This passes onmaintoo; it guards theerrors=choice.The first two fail on
main:With this change:
The 209 failures all happen on this Windows machine with or without this change. Re-running the same 42 test files on unmodified
main(1e933c4) fails exactly the same 209 tests, and none fails only on this branch. They're mostly the bash-vs-Python script parity tests and symlink tests (creating symlinks on Windows needs Developer Mode or admin rights). Nothing in the workflow run/state tests changed.Sample project, using the real CLI on Windows 11 (10.0.26200) with Python 3.12. The workflow has
name: "Demo Hello Pipeline (教学演示版)"and a shell step, run asspecify workflow run demo.yml -i spec="演示:完整闭环":AI Disclosure
AI disclosure: Implemented with Claude Code (desktop app) using Claude Opus 5.5 at xhigh reasoning effort, in autonomous agent mode under my direction. I chose the issue and approved the plan; the agent wrote the code change, the tests and this PR description, and ran all of the testing above on my Windows machine. Commits carry an
Assisted-by:trailer.