Skip to content

fix: explicit UTF-8 encoding in similarity-labels fixture load - #620

Open
ruggbk wants to merge 1 commit into
AgoraDMV:developfrom
ruggbk:fix/windows-utf8-similarity-labels
Open

fix: explicit UTF-8 encoding in similarity-labels fixture load#620
ruggbk wants to merge 1 commit into
AgoraDMV:developfrom
ruggbk:fix/windows-utf8-similarity-labels

Conversation

@ruggbk

@ruggbk ruggbk commented Aug 12, 2026

Copy link
Copy Markdown

Related issue

Closes #[find the issue number, or leave blank if there isn't one]

What does this change?

Path.read_text() defaults to the system locale encoding on Windows (cp1252),
causing a UnicodeDecodeError when the fixture contains non-ASCII characters.
Pinning encoding="utf-8" matches the file's actual encoding on all platforms.

How to test

uv run pytest tests/test_similarity_labels.py — 9 passed, 6 xfailed on Windows.
No change in behavior on Linux/Mac (UTF-8 was already the default there).

To verify the bug exists without the fix, revert the change and run the same
command on a Windows machine.

Checklist

  • Linked the issue above (Closes #...)
  • Ran the CI gates locally and they pass
  • New or changed behavior has tests
  • For a bug fix: the test fails without the fix, and I ran it both ways to check

AI assistance

Written with Claude Sonnet 4.6 (Claude Code). Brandon reviewed and tested the result.

read_text() defaults to the system locale on Windows (cp1252), causing
a UnicodeDecodeError when the JSON fixture contains non-ASCII characters.
Pinning encoding="utf-8" matches the file's actual encoding on all platforms.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@willhea willhea 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.

Thanks for the contribution — the core read_text(encoding="utf-8") fix looks right. I have a few small requests before we merge this.

  1. Please open an issue for this bug and link it with Closes #... in the PR. The issue can be bare minimum; we mainly want the repository/project history to connect the bug to the fix.

  2. Please complete the minimum PR-template requirements. In particular, replace the current issue placeholder, make sure the checklist reflects what was actually run/verified, and include the AI-assistance checklist item from the current template. The existing description and Windows reproduction are otherwise sufficient.

  3. Please make the matching change to the fixture generator. The reader now explicitly expects UTF-8, but scripts/build_similarity_labels.py currently writes the same fixture with:

_OUT.write_text(json.dumps(payload, indent=2, ensure_ascii=False) + "\n")

Because ensure_ascii=False preserves characters such as curly quotes literally, an encoding-less write_text() can write the fixture using the Windows locale encoding. That can produce a file which the new UTF-8 reader cannot read.

Please change it to:

_OUT.write_text(
    json.dumps(payload, indent=2, ensure_ascii=False) + "\n",
    encoding="utf-8",
)

I reproduced both failure directions: the committed UTF-8 fixture can fail when read as cp1252, and a cp1252-written version of the fixture fails when read explicitly as UTF-8. Explicit UTF-8 on both the generator and reader round-trips the content and bytes correctly.

Since you already have a Windows environment available, a sufficient final verification would be:

uv run python scripts/build_similarity_labels.py
git diff --exit-code tests/data/similarity_labels.json
uv run pytest tests/test_similarity_labels.py

No additional permanent test or Windows CI work requested for this PR. Once those items are addressed and CI is green, I don't see another substantive review round being necessary.

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.

2 participants