Files
LearningCircuit 6a1a6ae28d fix(settings): specify UTF-8 encoding for all file reads (#3797)
* fix(settings): specify UTF-8 encoding for all file reads (#3743)

On Windows, open() defaults to the system locale encoding (e.g. cp1252),
which causes JSON files containing non-ASCII characters to fail silently.
This left default_settings.json unloadable, resulting in only 14 of 27
search engines being available on a fresh install.

Add encoding="utf-8" to all read-mode open() calls across settings
loading, benchmarks, journal quality, security, and app factory.

* fix: add explicit UTF-8 encoding to all text-mode open() calls

Ensures consistent read/write encoding across Windows, Linux, and macOS.
Uses utf-8-sig for JSON config files to handle BOM-prefixed files from
Windows editors. Fixes read/write mismatch where files written without
explicit encoding (defaulting to cp1252 on Windows) were later read as
UTF-8.

Fixes #3743

* chore: add towncrier news fragment for PR #3797

* feat(hooks): add pre-commit check for missing encoding on open() calls

AST-based hook that flags text-mode open() calls without explicit
encoding=. Skips binary modes and dynamically-computed mode strings.
Prevents recurrence of Windows encoding failures like #3743.

* fix(cookiecutter): add encoding to open() calls in pre_prompt hook

* fix: correct pre-commit hook arg index and add missing read_text() encoding (#4083)

The check-open-encoding hook's _is_text_mode function was checking
call.args[0] (the file path) instead of call.args[1] (the mode) to
determine binary vs text mode, causing false positives for binary opens.

Also adds encoding="utf-8" to the read_text() call in
api/settings_utils.py that reads the same search engine JSON config
files affected by issue #3743 but via a different code path.

* fix: add encoding to sentinel Path.open/read_text and exclude tests/ from hook

Review of PR #3797 surfaced two Path-method calls in journal_quality/downloader.py
that the new check-open-encoding hook cannot catch (it only matches bare open()):

- sentinel.read_text() at line 353
- sentinel.open("x") at line 375

Both write/read a PID string — pure ASCII in practice, so this doesn't crash,
but it violates the explicit-encoding policy from #3743.

Also adds `exclude: ^tests/` to the check-open-encoding hook to match the
convention used by check-silent-cleanup and avoid spurious failures on test
fixtures that intentionally use bare open() to exercise file I/O.

* fix(hooks): exclude examples/ and scripts/ from check-open-encoding

CI runs pre-commit against all files, surfacing 19 pre-existing bare open()
calls in examples/ and scripts/. Those directories are out of scope for #3743
(the Windows-encoding bug is in the installed package under src/). Excluding
them lets the hook protect production code without blocking on legacy
demos/utilities. They can be cleaned up in a follow-up.

* fix(api): use utf-8-sig for bundled search engine JSONs (#4099)

`api/settings_utils.py` and `settings/manager.py` read the same bundled
JSON files (`defaults/settings/search_engines/*.json`) but with
different encodings — `manager.py` uses `utf-8-sig`, this path used
plain `utf-8`. If a file is ever re-saved with a BOM (e.g. Windows
Notepad), the two paths would diverge: `manager.py` would silently
strip the BOM, this path would raise `JSONDecodeError`.

Align both readers on `utf-8-sig`.

* fix(hooks): extend check-open-encoding to catch Path.open / read_text / write_text (#4100)

* fix(hooks): extend check-open-encoding to cover Path.open / read_text / write_text

The original hook only matched bare open(), missing the entire pathlib family.
That's exactly the bug class that slipped through PR #3797 — two
Path.open()/Path.read_text() calls in journal_quality/downloader.py had to be
fixed in a separate commit because the hook couldn't catch them. Without this
extension, the next regression of the same shape ships silently.

Hook now flags:
  * bare open() with text mode and no encoding= (unchanged)
  * <expr>.read_text() / <expr>.write_text() with no encoding= kwarg —
    pathlib-only methods, low false-positive risk
  * <expr>.open(<mode>, ...) only when the first positional is a constant
    string that looks like a real file mode (r/w/x/a + optional t/+, max 3
    chars) and contains no "b" — narrow enough to skip tarfile.open(path) /
    zipfile.ZipFile().open(name) and other non-file .open() methods

Also handles **kwargs spreads conservatively: treats them as "encoding may be
present" to avoid false positives on `open(path, **opts)`.

Fixed 8 read_text() call sites that the extended hook surfaced, all in
.pre-commit-hooks/ utilities that parse source files. Same Windows-encoding
risk as the production fixes from #3797.

* fix(hooks): inspect mode= kwarg, not just positional, in check-open-encoding

The hook only looked at the positional mode slot, so open(f, mode="rb") was
treated as text (positional slot empty → default "r") and flagged for missing
encoding. A developer "fixing" the spurious warning by adding encoding="utf-8"
would hit a runtime ValueError: binary mode doesn't take an encoding argument.

Extract _get_mode_arg(call, positional_index) and use it for both bare open()
and <expr>.open() — checks the positional slot first, then falls back to the
mode= kwarg. Same logic for both, so the bug can't recur on one branch.

Caught by friendly AI reviewer on #3797.

* fix(hooks): flag bare path.open() the same way bare open(f) is flagged

The .open() branch required a non-None mode arg, so path.open() with no
arguments slipped through. Bare open(f) is correctly flagged when mode is
omitted (defaults to "r"); bare path.open() has the identical risk and should
match.

Now flags when either:
  * mode is omitted entirely (defaults to text "r"), or
  * mode is a constant string that shape-matches a file mode and isn't binary

False-positive surface stays narrow: a no-arg .open() on a non-Path receiver
(e.g. a domain connection object) would be a rare case and the fix is to add
encoding= or refactor — same trade-off the reviewer accepted.

Caught by friendly AI reviewer on #4100.

* fix(hooks): correct _is_text_mode_at return type to bool | None

The function returns None when the mode can't be determined statically
(e.g. a variable mode), but was annotated -> bool. Mypy could flag this
on a stricter config; the fix aligns the annotation with the documented
behavior.

Caught by friendly AI reviewer.

* fix(hooks): add encoding to check-codeowners-sync read_text calls

Added to main after this branch forked; surfaced by CI after merging main
into this branch. Same Windows-encoding risk as the other hook utilities
fixed in this PR.
2026-05-18 19:10:45 +02:00
..