fix: survive a non-UTF-8 stdout and stop silently dropping screenshots

[chinmayajha]

Closes #587

Two independent Windows-correctness bugs, one commit each. Neither mechanism is
Windows-specific, so both regression suites run on every platform — no
skipif, no Windows runner required.

1. Any command dies when stdout cannot encode a character it prints

optics doctor aborted before printing a single row:

$ PYTHONIOENCODING=cp1252 optics doctor > out.txt
Error: 'charmap' codec can't encode character '\U0001fa7a' in position 0: character maps to <undefined>
$ echo $?   # 3, and out.txt is empty

That command reproduces the Windows CI failure verbatim on macOS: cp1252 is
what Python picks for a redirected sys.stdout on Windows, and rich's
safe_box degrades the box drawing but does not transcode text. Reduced:

import io
from rich.console import Console
stream = io.TextIOWrapper(io.BytesIO(), encoding="cp1252", errors="strict")
Console(file=stream, force_terminal=False).print("[bold]\U0001fa7a optics doctor[/bold]\n")
# UnicodeEncodeError

Why one choke point rather than 13 call sites

13 shipped modules print characters cp1252 has no representation for —
doctor.py (🩺 and the ✅/⚠️/❌ on every row), onboarding.py, error.py,
runner/printers.py, live_tui.py, quickstart.py, setup.py,
config_manager.py, ai_self_heal.py, async_utils.py, live.py and the two
Playwright engines. Editing the reported line only moves the crash to the next
one, and the set grows every time someone adds a glyph.

So the fix relaxes the stream's error handler once, in
optics_framework/helper/console_encoding.py, called from the package root.
rich resolves sys.stdout at write time and reconfigure mutates the stream
in place, which means the existing module-level Console() objects,
rich.get_console(), rich.prompt, the prompt_toolkit TUI and plain print
are all covered without being rebuilt or re-plumbed.

Alternatives considered and rejected:

• A shared console factory. Covers only the four explicit Console()
  sites. It misses rich.get_console() (error.py, printers.py),
  rich.prompt.Confirm/Prompt, RichHandler's log output and plain
  print — so it is not a choke point, just a fifth thing to keep in sync.
• Reconfiguring in helper/cli.py:main. Only the CLI reaches it, and only
  after argparse — cli.py's own import guard can render the "Cannot start"
  panel before main() runs, and the SDK, Robot Framework library,
  optics serve and optics mcp never reach it at all. All eight entry
  points do reach the package root.
• rich's own options. safe_box is about box-drawing characters;
  emoji is about :shortcode: substitution. Neither transcodes literal text,
  and rich re-raises the UnicodeEncodeError after printing a hint.

The eager import added to optics_framework/__init__.py is stdlib-only, so the
PEP 562 laziness the root exists to preserve is untouched, and
helper/abort.py needs no change — it stays stdlib-only and is already
downstream of the root.

The encoding itself is deliberately left alone. A stream that can already
encode what we print, or that has a non-raising error handler (someone's
explicit PYTHONIOENCODING=cp1252:backslashreplace), is not touched at all —
so a working pipeline keeps receiving exactly the bytes it did before, and only
characters it never had a representation for become ?. Losing an emoji is
acceptable; losing the whole diagnostic is not.

Verified by driving seven of the affected modules' console paths through one
cp1252 stream: 7/7 raise on main, 0/7 after. PYTHONIOENCODING=cp1252 and
PYTHONIOENCODING=ascii (the LC_ALL=C Linux case) both give a complete
optics doctor report and exit 0; UTF-8 output is byte-identical to before.

2. Screenshots silently dropped when the filename is rejected

utils.save_screenshot interpolated the caller-supplied time_stamp into the
filename unsanitised. utils.get_timestamp() returns ISO-8601
(2026-09-22T16:51:50.397462+05:30), whose colons are reserved on Windows, and
api/verifier.py plus both annotated-result helpers in api/action_keyword.py
pass exactly that.

The asymmetry is what kept it hidden: the default timestamp branch already
used a portable %H-%M-%S-%f, and name was already sanitised. Only the
passed-in timestamp was not — so the bug misses the common path and hits
precisely the captures that carry detection annotations. The fix closes that
one gap where the filename is composed, rather than inventing a new scheme.

And it failed silently, which is the worse half. cv2.imwrite reports a
rejected path by returning False, not by raising, and the call sat inside a
try/except, so the capture vanished while the run still exited 0 and reported
a pass. A test that loses its evidence and claims success is worse than one
that fails outright. The return is now checked and a failed write is logged at
warning — loud enough to see in a normal run, but still non-fatal, because a
screenshot is diagnostic output and should not take down a passing test.

save_screenshot now also returns the path it wrote. optics live's
/screenshot used that to replace its reconstruction of the filename it
assumed would be used; it now reports the file that exists, and raises rather
than handing back a path to nothing.

Tests

tests/units/helpers/test_console_encoding.py (9) and
tests/units/common/test_save_screenshot.py (11). 4 of the 9 and 7 of the 11
fail on origin/main today; the rest are no-op guarantees (UTF-8 streams
untouched, non-raising handlers untouched, non-reconfigurable streams ignored,
an empty image still raises) that must hold on both sides. One of them scans
the shipped source for every cp1252-unencodable character and prints the lot
through a cp1252 stream, so the "whole family" claim keeps being checked as
modules add glyphs.

Full unit suite green (1378 passed, 2 pre-existing xfail), pre-commit clean on
all six touched files.

Relationship to #582

#582 (Windows CI) is blocked on bug 1 — optics doctor is unrunnable there —
and will lose these failures once this merges. #582 has further Windows-only
failures of its own (test-side path-separator and tty assumptions) that this PR
does not touch.

Confidence Score: 4/5

The PR should not merge until package imports stop globally altering embedding applications' output semantics and the explicit documentation rule is satisfied.

Findings

1.  Import Changes Process Output ▶
2.  Tests Add Redundant Documentation ▶

Fix with agent prompt

### Issue 1
optics_framework/__init__.py:36
Importing any `optics_framework` submodule now reconfigures the process-wide stdout and stderr streams. In a host using `ascii:surrogateescape` to preserve arbitrary bytes, this changes the handler to `replace`, so output such as `\udc80` changes from byte `0x80` to `?`. Unrelated host output can therefore be silently corrupted merely by importing the SDK. Limit this mutation to Optics-owned command or output boundaries rather than package initialization.

### Issue 2
tests/units/common/test_save_screenshot.py:1-9
This module docstring repeats behavior already clear from the test names and assertions. Similar redundant prose appears in `test_console_encoding.py` and several test-method docstrings. This violates the repository directive to omit unnecessary comments and test docstrings, so the redundant documentation must be removed before merging.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR addresses two needed portability defects: non-UTF console streams aborting on framework glyphs, and screenshot writes failing for ISO-8601 filenames on Windows.

• Adds centralized console-stream fallback handling during package initialization.
• Sanitizes timestamp filename components and checks cv2.imwrite results.
• Makes live screenshot capture return the actual saved path and fail when no file was written.
• Adds cross-platform regression coverage for both behaviors.
• Package initialization currently changes output semantics for the entire embedding process, and the new tests include redundant documentation prohibited by repository guidance.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Import optics_framework] --> B[Probe stdout and stderr encoding]
  B -->|Probe encodable or handler already non-raising| C[Leave stream unchanged]
  B -->|Strict incompatible encoding| D[Reconfigure process stream to replace]
  E[Capture screenshot] --> F[Sanitize timestamp component]
  F --> G[cv2.imwrite]
  G -->|true| H[Return actual path]
  G -->|false or exception| I[Warn and return None]
  I --> J[Live capture raises E0303]

Loading

_{Reviews (1) · Last reviewed commit: "fix: stop dropping screenshots whose fil..."}

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[greptile-apps]

Import Changes Process Output

Importing any optics_framework submodule now reconfigures the process-wide stdout and stderr streams. In a host using ascii:surrogateescape to preserve arbitrary bytes, this changes the handler to replace, so output such as \udc80 changes from byte 0x80 to ?. Unrelated host output can therefore be silently corrupted merely by importing the SDK. Limit this mutation to Optics-owned command or output boundaries rather than package initialization.

Prompt To Fix With AI

This is a comment left during a code review.
Path: optics_framework/__init__.py
Line: 36

Comment:
**Import Changes Process Output**

Importing any `optics_framework` submodule now reconfigures the process-wide stdout and stderr streams. In a host using `ascii:surrogateescape` to preserve arbitrary bytes, this changes the handler to `replace`, so output such as `\udc80` changes from byte `0x80` to `?`. Unrelated host output can therefore be silently corrupted merely by importing the SDK. Limit this mutation to Optics-owned command or output boundaries rather than package initialization.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Tests Add Redundant Documentation

This module docstring repeats behavior already clear from the test names and assertions. Similar redundant prose appears in test_console_encoding.py and several test-method docstrings. This violates the repository directive to omit unnecessary comments and test docstrings, so the redundant documentation must be removed before merging.

Context Used: Don't just check whether PR is valid or not. Also check whether the PR is needed at all or not. The code should not have unnecessary code comments. SonarQube rules have to be followed too. Tests need not have unnecessary docstrings. Follow CLAUDE.md ... (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/units/common/test_save_screenshot.py
Line: 1-9

Comment:
**Tests Add Redundant Documentation**

This module docstring repeats behavior already clear from the test names and assertions. Similar redundant prose appears in `test_console_encoding.py` and several test-method docstrings. This violates the repository directive to omit unnecessary comments and test docstrings, so the redundant documentation must be removed before merging.

**Context Used:** Don't just check whether PR is valid or not. Also check whether the PR is needed at all or not. The code should not have unnecessary code comments. SonarQube rules have to be followed too. Tests need not have unnecessary docstrings. Follow CLAUDE.md ... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!