Skip to content

fix(detect): stop silently dropping rules from a non-UTF-8 ignore file (#2798) - #2799

Closed
abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:fix/ignore-file-encoding
Closed

fix(detect): stop silently dropping rules from a non-UTF-8 ignore file (#2798)#2799
abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:fix/ignore-file-encoding

Conversation

@abhay-codes07

Copy link
Copy Markdown
Contributor

Fixes #2798.

The bug

.gitignore, .graphifyignore and $GIT_DIR/info/exclude were read with errors="ignore":

for raw in ignore_file.read_text(encoding="utf-8-sig", errors="ignore").splitlines():

errors="ignore" turns a mis-encoded byte into no byte. An ignore file saved in the host ANSI codepage — Notepad's historical default on Windows, and still what Set-Content writes without -Encoding — is not valid UTF-8, so a rule reading Orçamento/ (cp1252 Or\xe7amento/) decoded to the pattern Oramento/.

That matches nothing, and nothing said so:

--- .graphifyignore saved as utf-8 ---
  raw bytes            : b'Or\xc3\xa7amento/\n'
  excluded as intended : True
  files scanned        : ['main.py']

--- .graphifyignore saved as cp1252 ---
  raw bytes            : b'Or\xe7amento/\n'
  excluded as intended : False
  files scanned        : ['contrato.py', 'main.py']
  *** the ignore rule silently did nothing ***

This is the failure mode test_graphifyignore_matches_nfd_path_with_nfc_pattern already warns about in its own docstring — "the rule silently does nothing — the files get scanned, and docs/PDFs are sent to an LLM despite an explicit exclusion" — reached by a different route. An exclusion that fails open is worth more than a tidy decode: the whole reason someone writes Orçamento/ is to keep that directory out of the corpus.

Change

_read_ignore_text tries UTF-8 (BOM-tolerant) first, since that is the format every other reader here assumes and what the docs describe. Only if that fails does it fall back to the host encoding, then to latin-1, which cannot fail and maps every byte to a codepoint.

A rule spelled in some third encoding still comes out wrong, but it comes out whole rather than being silently truncated to something that matches nothing — and a one-time warning names the file, so it is fixable instead of invisible:

[graphify] WARNING: <path>/.graphifyignore is not valid UTF-8; read it as cp1252
instead. Re-save it as UTF-8 — patterns with non-ASCII characters may not match
as written.

Decoding still never raises, which was the previous contract and matters here — a scan must not die on a stray byte in an ignore file. test_decoding_never_raises_on_arbitrary_bytes feeds it all 256 byte values to pin that.

I kept UTF-8 as the preferred encoding rather than guessing per-file: changing the documented format would be a much bigger decision than fixing a silent failure, and every ignore file that is already valid UTF-8 decodes on the first branch exactly as before.

Two existing tests were failing on Windows for a related reason

test_graphifyignore_matches_nfc_path_with_nfd_pattern and its mirror wrote the ignore file through write_text with no encoding=, so they emitted the locale codepage:

  • one raised UnicodeEncodeError outright, because the NFD form's combining cedilla (U+0327) has no cp1252 representation;
  • the other wrote cp1252 bytes that the reader then had to guess at.

Both now pass encoding="utf-8". That is a two-character fix, but it is the reason the product bug above stayed invisible: the tests that would have caught it could not run.

Tests

tests/test_ignore_file_encoding.py (12 tests). They write the ignore file as bytes rather than through write_text, so they pin the decoding behaviour on every platform rather than only where cp1252 happens to be the default — these have teeth on Linux CI, not just Windows.

Reverting _read_ignore_text to the old one-liner and keeping the tests fails 3, on either platform:

FAILED tests/test_ignore_file_encoding.py::test_ansi_encoded_rule_still_excludes
FAILED tests/test_ignore_file_encoding.py::test_ansi_encoded_rule_warns_once_naming_the_file
FAILED tests/test_ignore_file_encoding.py::test_no_byte_is_dropped_from_a_mis_encoded_file

The existing NFC/NFD normalisation guarantee is re-pinned through the new decode path in both directions, so this cannot quietly undo #1226's work.

Validation

Windows 11, Python 3.12, branched off 4fca621 (0.9.44).

  • Full suite: 20 failed, 4469 passed -> 18 failed, 4482 passed.
  • The two NFC/NFD tests move from fail to pass; no other failure changes state. The remaining 18 are pre-existing Windows failures (symlink privileges, FIFO/socket fixtures, and similar), unrelated to this change.

.gitignore, .graphifyignore and $GIT_DIR/info/exclude were read with
errors="ignore", which turns a mis-encoded byte into no byte. An ignore file
saved in the host ANSI codepage -- Notepad's historical default on Windows, and
still what Set-Content writes without -Encoding -- is not valid UTF-8, so a rule
reading `Orcamento/` (cp1252 `Or\xe7amento/`) decoded to `Oramento/`. That
matches nothing, and nothing said so: the directory was scanned despite an
explicit exclusion. For a rule covering documents or PDFs that means they reach
the semantic pass anyway, which is the same silent-exclusion-failure the NFC/NFD
tests already warn about in prose, reached by a different route.

_read_ignore_text now tries UTF-8 (BOM-tolerant) first, since that is the format
every other reader here assumes, and only on failure falls back to the host
encoding and then latin-1, which cannot fail and maps every byte to a codepoint.
A rule spelled in some third encoding still comes out wrong, but it comes out
whole, and a one-time warning names the file so it can be fixed. Decoding still
never raises, matching the previous contract.

Also fixes the two NFC/NFD tests in test_detect.py, which wrote .graphifyignore
through write_text with no encoding= and so emitted the locale codepage: one
raised UnicodeEncodeError on the combining cedilla, the other wrote cp1252 bytes
the reader then had to guess at. Both have been failing on Windows.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

This PR changes how ignore files (.gitignore, .graphifyignore, and $GIT_DIR/info/exclude) are read in graphify/detect.py. It introduces a new _read_ignore_text helper that attempts UTF-8 (BOM-tolerant) decoding first, then falls back to the host's preferred encoding and finally latin-1, emitting a one-time stderr warning naming the file when a non-UTF-8 fallback is used, and replaces the prior read_text(..., errors="ignore") calls at the three read sites. It also adds a new test file tests/test_ignore_file_encoding.py covering the decode behavior (ANSI/cp1252 rules, warnings, BOM stripping, arbitrary bytes, empty files, and NFC/NFD normalization) and updates two existing tests in tests/test_detect.py to pass an explicit encoding="utf-8" to their write_text calls. The surface area is limited to ignore-file reading and its tests.

No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1546 functions depend on the 501 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 463 callers, 41 callees
  • new: _rebuild_code() — 95 callers, 51 callees
  • new: detect() — 106 callers, 15 callees
  • new: save_manifest() — 34 callers, 11 callees
  • new: extract_files_direct() — 17 callers, 20 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: extract_corpus_parallel() — 26 callers, 10 callees
  • new: dispatch_command() — 2 callers, 117 callees
  • …and 25 more

Verification — 1546 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: 804 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify \_load\_dir\_own\_ignore.

The verifier did not have enough to check \_load\_dir\_own\_ignore, 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: parameter `d` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_load\_graphifyignore.

The verifier did not have enough to check \_load\_graphifyignore, 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: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set

· 33 more finding(s) on lines outside this diff (see the check run).

safishamsi added a commit that referenced this pull request Aug 17, 2026
…ge (#2798)

The #2799 fallback (utf-8 -> host codepage -> latin-1) turned a BOM'd UTF-16
ignore file (what PowerShell Set-Content / Notepad 'Unicode' write) into
NUL-laden mojibake via latin-1, so its rules matched nothing. Detect the
UTF-16 BOM and decode as utf-16 before the latin-1 fallback. Adds an
end-to-end UTF-16 exclusion test and a direct no-NUL-garbage test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@safishamsi

Copy link
Copy Markdown
Collaborator

Landed in v0.9.46, just published to PyPI. Cherry-picked onto v8 with your authorship preserved in the commit, so both the change and the credit are in the history. Thanks @abhay-codes07. Closing since it is now released.

@safishamsi safishamsi closed this Aug 17, 2026
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.

A non-UTF-8 .graphifyignore silently loses its non-ASCII rules: errors="ignore" turns an exclusion into a pattern that matches nothing

3 participants