Repository navigation
fix(extract): decode UTF-16 and non-UTF-8 source files before parsing - #4146
rohit-jsfreaky wants to merge 2 commits into
Conversation
tree-sitter reads its input as UTF-8 and the extractors passed it raw file bytes. A UTF-16 file (Visual Studio "Unicode", PowerShell 5.1 Out-File) produced only its file node, and a Windows-1252 identifier was cut at its first non-UTF-8 byte (CaféOrder became Order), with no warning either way. Route every source read in extraction through _read_source_bytes / _read_source_text: valid UTF-8 (BOM or not) is passed through byte for byte, BOM'd UTF-16/UTF-32 is decoded exactly, and anything else is read as cp1252 then latin-1 with a one-time warning naming the file. The fallback is fixed, not the host locale, so node ids do not depend on the machine. Secondary passes re-reading a file pass warn=False. Three Spock/Catch2 reads that had no encoding= at all now go through it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8gwEpjajiWHVW3c9smuKf
|
Thanks for the pull request, @rohit-jsfreaky. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. PR-changed functions: 18/55 verified (0 proven, 18 may-equivalent, 0 distinguished) · 37 not verified (7 unsupported, 30 over cap).
Not verified on this run: \_augment\_cpp\_string\_tests (unsupported), \_extract\_js\_rationale (unsupported), \_extract\_python\_rationale (unsupported), \_extract\_spock\_fallback (unsupported), \_is\_spock\_file (unsupported), \_rescue\_js\_dynamic\_imports (unsupported), \_extract\_generic (unsupported); 30 changed function(s) beyond the cap and 0 skipped when the time budget ran out.
Graphify review — findings
Adds _read_source_bytes and _read_source_text and routes extractors through them, so tree-sitter and the regex extractors always receive UTF-8. Valid UTF-8 passes through byte-for-byte, so ids and offsets don't change, and BOM-marked UTF-16/UTF-32 files are transcoded instead of yielding no nodes. Anything else is decoded as cp1252, falling back to latin-1, with a once-per-file stderr warning that secondary re-read passes suppress via warn=False.
Worth a look
- unit_source_text decoding may no longer match _file_to_text, so slice offsets can drift —
graphify/file_slice.py:80· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 4577 functions depend on the 1199 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 780 callers, 50 callees - new:
_rebuild_code()— 149 callers, 56 callees - new:
build_from_json()— 233 callers, 20 callees - new:
build_merge()— 85 callers, 14 callees - new:
_extract_generic()— 18 callers, 32 callees - new:
extract_bash()— 52 callers, 11 callees - new:
extract_js()— 87 callers, 5 callees - new:
extract_files_direct()— 17 callers, 20 callees - …and 144 more — each is listed as a finding
Verification — 4577 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 4118 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
335 of 335 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_analyze.py— impact, full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— impact, full-run-safetytests/test_astro_import_ids.py— impact, full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_blade_extractor.py— impact, full-run-safetytests/test_build.py— impact, full-run-safetytests/test_build_located_semantic_identity.py— impact, full-run-safetytests/test_build_merge_dedup_scope.py— impact, full-run-safetytests/test_build_merge_hyperedges_and_prune.py— impact, full-run-safetytests/test_build_merge_shrink_guard.py— impact, full-run-safetytests/test_builtin_global_type_refs.py— impact, full-run-safetytests/test_cache.py— impact, full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_cargo_missing_manifest.py— full-run-safetytests/test_carried_hyperedge_remap.py— impact, full-run-safetytests/test_case_sensitive_resolution.py— impact, full-run-safetytests/test_charmap_encoding.py— impact, full-run-safetytests/test_chunking.py— impact, full-run-safetytests/test_cjs_module_extension.py— impact, full-run-safetytests/test_claude_cli_backend.py— impact, full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— impact, full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— impact, full-run-safetytests/test_cluster_exclude_hubs.py— full-run-safetytests/test_cobol_extractor.py— impact, full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— impact, full-run-safetytests/test_corrupt_graph_json.py— impact, full-run-safetytests/test_cpp_method_declarations.py— impact, full-run-safetytests/test_cpp_nested_and_cli.py— impact, full-run-safetytests/test_cpp_objc_cross_file_calls.py— impact, full-run-safetytests/test_cpp_preprocess.py— full-run-safety- … and 285 more
changed code file(s) with no mapped test (
graphify/extractors/blade.py,graphify/extractors/cobol.py,graphify/extractors/erlang.py,graphify/extractors/solidity.py,graphify/extractors/vbnet.py) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify \_augment\_cpp\_string\_tests.
The verifier did not have enough to check \_augment\_cpp\_string\_tests, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: the input domain has 59 values but only 24 distinct were tested — a small finite domain must be EXHAUSTED, not sampled (an untested input could invert the result)
Could not verify: Could not verify \_extract\_js\_rationale.
The verifier did not have enough to check \_extract\_js\_rationale, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: the input domain has 59 values but only 24 distinct were tested — a small finite domain must be EXHAUSTED, not sampled (an untested input could invert the result)
Could not verify: Could not verify \_extract\_python\_rationale.
The verifier did not have enough to check \_extract\_python\_rationale, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: the input domain has 59 values but only 24 distinct were tested — a small finite domain must be EXHAUSTED, not sampled (an untested input could invert the result)
Could not verify: Could not verify \_extract\_spock\_fallback.
The verifier did not have enough to check \_extract\_spock\_fallback, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: the input domain has 59 values but only 24 distinct were tested — a small finite domain must be EXHAUSTED, not sampled (an untested input could invert the result)
Could not verify: Could not verify \_is\_spock\_file.
The verifier did not have enough to check \_is\_spock\_file, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: the input domain has 59 values but only 24 distinct were tested — a small finite domain must be EXHAUSTED, not sampled (an untested input could invert the result)
Could not verify: Could not verify \_rescue\_js\_dynamic\_imports.
The verifier did not have enough to check \_rescue\_js\_dynamic\_imports, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: the input domain has 59 values but only 24 distinct were tested — a small finite domain must be EXHAUSTED, not sampled (an untested input could invert the result)
No difference found (not proven): No behavior difference found in \_xaml\_codebehind\_symbols (not a proof).
The verifier ran both versions of \_xaml\_codebehind\_symbols on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in \_xaml\_communitytoolkit\_members (not a proof).
The verifier ran both versions of \_xaml\_communitytoolkit\_members on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_astro (not a proof).
The verifier ran both versions of extract\_astro on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_cpp (not a proof).
The verifier ran both versions of extract\_cpp on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_js (not a proof).
The verifier ran both versions of extract\_js on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_php (not a proof).
The verifier ran both versions of extract\_php on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_svelte (not a proof).
The verifier ran both versions of extract\_svelte on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_vue (not a proof).
The verifier ran both versions of extract\_vue on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_apex (not a proof).
The verifier ran both versions of extract\_apex on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_bash (not a proof).
The verifier ran both versions of extract\_bash on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_blade (not a proof).
The verifier ran both versions of extract\_blade on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_cobol (not a proof).
The verifier ran both versions of extract\_cobol on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_commonlisp (not a proof).
The verifier ran both versions of extract\_commonlisp on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_dart (not a proof).
The verifier ran both versions of extract\_dart on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_dm (not a proof).
The verifier ran both versions of extract\_dm on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_dmf (not a proof).
The verifier ran both versions of extract\_dmf on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_dmm (not a proof).
The verifier ran both versions of extract\_dmm on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in extract\_elixir (not a proof).
The verifier ran both versions of extract\_elixir on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: parameter `config` is annotated `LanguageConfig` — outside the synthesizable primitive/collection set
· 10 grounded finding(s) anchored inline below; 142 more finding(s) on lines outside this diff (see the check run).
| from graphify.extractors.base import _file_stem, _make_id, _read_source_text | ||
|
|
||
|
|
||
| def extract_apex(path: Path) -> dict: |
There was a problem hiding this comment.
extract_apex()
15 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| @@ -98,7 +98,7 @@ def _starts_inside_string(code: str, offset: int) -> bool: | |||
|
|
|||
| def extract_cobol(path: Path) -> dict: | |||
There was a problem hiding this comment.
extract_cobol()
fans out to 8 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| from graphify.extractors.base import _file_stem, _make_id, _read_source_text | ||
|
|
||
|
|
||
| def extract_dart(path: Path) -> dict: |
There was a problem hiding this comment.
extract_dart()
fans out to 8 callees (efferent coupling); 13 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| ) | ||
|
|
||
|
|
||
| def extract_dm(path: Path) -> dict: |
There was a problem hiding this comment.
extract_dm()
fans out to 7 callees (efferent coupling); 15 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| @@ -436,7 +442,7 @@ def extract_dmm(path: Path) -> dict: | |||
| def extract_dmf(path: Path) -> dict: | |||
There was a problem hiding this comment.
extract_dmf()
7 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| ) | ||
|
|
||
|
|
||
| def extract_elixir(path: Path) -> dict: |
There was a problem hiding this comment.
extract_elixir()
fans out to 6 callees (efferent coupling); 14 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| from graphify.extractors.base import _file_stem, _make_id, _read_source_bytes, _read_text | ||
|
|
||
|
|
||
| def extract_ocaml(path: Path) -> dict: |
There was a problem hiding this comment.
extract_ocaml()
fans out to 8 callees (efferent coupling); 14 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| from graphify.extractors.base import _file_stem, _make_id, _read_source_text | ||
|
|
||
|
|
||
| def extract_lazarus_form(path: Path) -> dict: |
There was a problem hiding this comment.
extract_lazarus_form()
9 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| from graphify.extractors.base import _file_stem, _make_id, _read_source_bytes, _read_text | ||
|
|
||
|
|
||
| def extract_powershell(path: Path) -> dict: |
There was a problem hiding this comment.
extract_powershell()
fans out to 7 callees (efferent coupling); 21 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| from graphify.security import sanitize_metadata | ||
|
|
||
|
|
||
| def extract_razor(path: Path) -> dict: |
There was a problem hiding this comment.
extract_razor()
15 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
unit_source_text (slice measuring, cutting, bisecting) now decodes non-UTF-8 files, but llm._file_to_text, which sends a document whole when it fits, still read with utf-8/errors="replace". The two readers must agree: a small UTF-16 note reached the model as NUL-riddled text while a big one was sliced and read cleanly. Slice offsets themselves were consistent, since every slice path uses unit_source_text. Read whole documents and their token estimate through _read_source_text too. Valid UTF-8 gives exactly the text it did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8gwEpjajiWHVW3c9smuKf
|
Checked the
Fixed in 56f11b2: The coupling notes on the extractors are the one added call to the shared reader. |
…#4146, #4145) tree-sitter reads UTF-8, but extractors handed it raw file bytes, so a UTF-16 file yielded zero nodes and a cp1252 name like CaféOrder was truncated. Decode via a shared reader that returns UTF-8 bytes unchanged (ids/offsets/cache keys for working files are byte-identical) and re-encodes UTF-16/UTF-32/non-UTF-8 using a deterministic cp1252->latin-1 fallback. The semantic whole-file and slice readers now share the same decoder. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Landed in v0.9.78 via an authorship-preserving cherry-pick, so your commit is on |
What does this PR do?
Fixes #4145.
tree-sitter reads its input as UTF-8, and every extractor handed it the raw file bytes. A UTF-16 file (Visual Studio "Unicode", Windows PowerShell 5.1
Out-File) produced only its file node, and a Windows-1252 identifier was cut at its first non-UTF-8 byte (CaféOrder→Order). Both were silent.This adds two helpers in
extractors/base.pyand routes every source read in extraction through them:_read_source_bytes(path, *, warn=True): valid UTF-8, with or without a BOM, is returned byte for byte (anisascii()fast path, then a strict UTF-8 check). A UTF-16/UTF-32 file with a BOM is decoded exactly and re-encoded as UTF-8. Anything else is decoded as cp1252, then latin-1 (which cannot fail), and a warning names the file once per process._read_source_text(path, *, warn=True): the same, asstr. For valid UTF-8 it returns exactly whatpath.read_text(encoding="utf-8")did, including universal-newline translation and a kept BOM, so the regex extractors see the same text as before.59 read sites in 34 files now use them: 31 tree-sitter
read_bytes()reads and 28read_text(..., errors="replace")reads. Two of those are in the semantic pass:llm._file_to_text(a document sent whole) and its token estimate now use the same reader asfile_slice.unit_source_text(a document cut into slices), so a document reads the same whichever path it takes. Secondary passes that re-read a file its extractor already read (cross-file resolution, rationale, Spock/Catch2 fallbacks, XAML code-behind) passwarn=False, so one bad file gives one warning. That includes three reads (_is_spock_file,_extract_spock_fallback,_augment_cpp_string_tests) that had noencoding=at all and so depended on the host locale.Left as they are on purpose: binary readers (
.dmi, Delphi binary forms), XML project files that declare their own encoding (.csproj,.slnx,.xaml, Lazarus.lpk), header sniffers (_is_objc_header,_is_cpp_header), and JSON/YAML config reads (tsconfig,package.json,pnpm-workspace.yaml).Type of change
Verification & Invariants
Invariant: the same source text gives the same graph whatever encoding the file was saved in, and every file that was valid UTF-8 gives exactly the graph it gave before.
Valid UTF-8 is never re-encoded:
_read_source_bytesreturns the originalbytesobject, so node ids, byte offsets and source locations of existing files cannot move. Checked on all 453.pyfiles of this repo (313 contain non-ASCII): every one byte-identical toread_bytes().The fallback is a fixed
("cp1252", "latin-1"), notlocale.getpreferredencoding()._read_ignore_textuses the host encoding, which is fine for ignore rules that never leave the machine, but node ids are persisted and shared, and CONTRIBUTING asks that extraction not depend on ambient state. A test pins this by monkeypatching the locale.FF FE 00 00is both the UTF-32-LE BOM and a UTF-16-LE BOM followed by U+0000; a failed UTF-32 decode falls through to UTF-16 (tested).Line numbers are unchanged in every case (decoding never adds or removes a line), so
source_locationstays correct.Persisted state: the cache key is the hash of the file bytes and the cache directory carries the package version, so the empty/truncated results cached for non-UTF-8 files by older versions are not reused after upgrading. UTF-8 files hash the same and extract the same.
Cost: reading those 8.4 MB took 46 ms instead of 35 ms (best of 5).
Read the CONTRIBUTING.md guide.
Reproduced the issue and identified the invariant.
Made the smallest fix necessary.
Added a regression test (if bug fix) or isolated boundary test.
Kept the PR description synchronized with the final implementation.
Documented any limitations / unsupported cases explicitly.
Limitations: a file in a third legacy encoding (Shift-JIS, GBK, cp1251, ...) is still decoded as cp1252/latin-1. Its names come out whole but mis-spelled, and the warning names the file so it can be re-saved as UTF-8. UTF-16 without a BOM is not detected.
How was this tested?
New
tests/test_source_file_encoding.py, 42 tests:FF FE 00 00case; cp1252 read whole and warned once;warn=Falsesilent; latin-1 fallback for a byte cp1252 does not define; locale independence;_read_source_textequalsread_text(encoding="utf-8")for CRLF, lone CR and BOM.TEST_CASEis recovered.llm._file_to_textandfile_slice.unit_source_textreturn the same text.Before the fix (upstream
5c7b847with only the helpers added, so the decoder tests can import), all 19 extraction tests fail; without thellm.pychange the UTF-16 and cp1252 reader-agreement cases fail; with the fix all 42 pass.Windows 11, Python 3.12.
Graphify-specific checklist
uv run python -m tools.skillgen --bless) when changing their source fragments. (not applicable, no skill sources changed)🤖 Generated with Claude Code