mirror of
https://github.com/semantica-agi/semantica.git
synced 2026-09-11 04:01:32 +00:00
ConsoleProgressDisplay wrote every progress frame to sys.stdout. Progress is diagnostic output, so stderr is the correct stream for it — tqdm and most progress renderers default there for the same reason — and stdout must stay clean for programs that carry a machine-readable protocol on it. The stdio MCP servers put newline-delimited JSON-RPC on stdout, where an interleaved progress bar makes a response body unparseable (#1134). ConsoleProgressDisplay now takes an optional stream, defaulting to stderr. The stream is resolved per write rather than captured at construction, so a later rebinding of sys.stderr (pytest capture, for instance) is honoured. All writes and the four bare flushes route through it, and the emoji-capability probe now inspects that stream rather than stdout, so a cp1252 stderr still degrades correctly. The existing cp1252 tests in tests/deduplication/test_deduplication.py patched sys.stdout to assert emoji auto-disabling; they now patch the stream progress is actually written to. Their intent is unchanged. Closes #1134 (point 1 only; the SEMANTICA_KG_PATH persistence and README items remain with @akaszubski)
161 lines
5.5 KiB
Python
161 lines
5.5 KiB
Python
"""Console progress must be written to stderr, never to stdout.
|
|
|
|
Progress is diagnostic output. Writing it to stdout corrupts any program that
|
|
carries a machine-readable protocol there — the stdio MCP servers put
|
|
newline-delimited JSON-RPC on stdout, and a progress bar interleaved with a
|
|
response body makes that response unparseable (#1134).
|
|
|
|
Both servers currently defend against this by setting
|
|
SEMANTICA_DISABLE_PROGRESS, and a console display is only attached when the
|
|
console is interactive. Those are containments, not the fix: they give up
|
|
progress output entirely, and every future entry point has to remember them.
|
|
Writing to the correct stream in the first place is what these tests pin.
|
|
"""
|
|
|
|
import importlib
|
|
import io
|
|
import sys
|
|
|
|
import pytest
|
|
|
|
|
|
@pytest.fixture
|
|
def progress_module(monkeypatch):
|
|
"""Import the real progress_tracker, bypassing a mocked sys.modules entry.
|
|
|
|
tests/test_extractors_dispatch.py assigns a MagicMock over
|
|
'semantica.utils.progress_tracker' at import time and never restores it, so
|
|
a plain module-level import here returns mocks when that file has already
|
|
run. Dropping the cached entry via monkeypatch re-imports the real module
|
|
and restores whatever was there afterwards.
|
|
"""
|
|
name = "semantica.utils.progress_tracker"
|
|
monkeypatch.delitem(sys.modules, name, raising=False)
|
|
module = importlib.import_module(name)
|
|
assert hasattr(module, "__file__"), "expected the real module, got a stand-in"
|
|
return module
|
|
|
|
|
|
@pytest.fixture
|
|
def display_cls(progress_module):
|
|
return progress_module.ConsoleProgressDisplay
|
|
|
|
|
|
@pytest.fixture
|
|
def make_item(progress_module):
|
|
def _make(**overrides):
|
|
defaults = dict(
|
|
module="kg",
|
|
submodule="Reasoner",
|
|
message="Inferring facts",
|
|
status="running",
|
|
total_items=10,
|
|
processed_items=3,
|
|
)
|
|
defaults.update(overrides)
|
|
return progress_module.ProgressItem(**defaults)
|
|
|
|
return _make
|
|
|
|
|
|
class TestProgressStreamDefaults:
|
|
def test_defaults_to_stderr(self, display_cls):
|
|
assert display_cls().stream is sys.stderr
|
|
|
|
def test_default_is_not_stdout(self, display_cls):
|
|
"""The whole point: stdout stays clean for protocol traffic."""
|
|
assert display_cls().stream is not sys.stdout
|
|
|
|
def test_stream_follows_rebinding(self, display_cls, monkeypatch):
|
|
"""Resolved per write, so pytest capture and later rebinds are honoured."""
|
|
display = display_cls()
|
|
replacement = io.StringIO()
|
|
monkeypatch.setattr(sys, "stderr", replacement)
|
|
assert display.stream is replacement
|
|
|
|
def test_explicit_stream_overrides_the_default(self, display_cls):
|
|
buffer = io.StringIO()
|
|
assert display_cls(stream=buffer).stream is buffer
|
|
|
|
|
|
class TestProgressWritesGoToTheStream:
|
|
def test_update_writes_to_the_configured_stream(self, display_cls, make_item):
|
|
buffer = io.StringIO()
|
|
display = display_cls(stream=buffer, use_emoji=False, update_interval=0.0)
|
|
|
|
display.update(make_item())
|
|
|
|
assert buffer.getvalue(), "progress should have been rendered"
|
|
|
|
def test_update_writes_nothing_to_stdout(self, display_cls, make_item, monkeypatch):
|
|
"""Regression guard for #1134: stdout must stay untouched."""
|
|
fake_stdout = io.StringIO()
|
|
monkeypatch.setattr(sys, "stdout", fake_stdout)
|
|
buffer = io.StringIO()
|
|
|
|
display = display_cls(stream=buffer, use_emoji=False, update_interval=0.0)
|
|
display.update(make_item())
|
|
display.clear()
|
|
|
|
assert fake_stdout.getvalue() == "", (
|
|
f"console progress leaked to stdout: {fake_stdout.getvalue()!r}"
|
|
)
|
|
|
|
def test_default_display_writes_nothing_to_stdout(
|
|
self, display_cls, make_item, monkeypatch
|
|
):
|
|
"""Same guard without an explicit stream, i.e. the real default path."""
|
|
fake_stdout = io.StringIO()
|
|
fake_stderr = io.StringIO()
|
|
monkeypatch.setattr(sys, "stdout", fake_stdout)
|
|
monkeypatch.setattr(sys, "stderr", fake_stderr)
|
|
|
|
display = display_cls(use_emoji=False, update_interval=0.0)
|
|
display.update(make_item())
|
|
|
|
assert fake_stdout.getvalue() == ""
|
|
assert fake_stderr.getvalue(), "progress should have gone to stderr"
|
|
|
|
def test_clear_flushes_the_stream_not_stdout(self, display_cls, monkeypatch):
|
|
flushed = []
|
|
|
|
class RecordingStream(io.StringIO):
|
|
def flush(self):
|
|
flushed.append("stream")
|
|
|
|
class ExplodingStdout(io.StringIO):
|
|
def flush(self): # pragma: no cover - fails the test if reached
|
|
raise AssertionError("progress must not flush stdout")
|
|
|
|
monkeypatch.setattr(sys, "stdout", ExplodingStdout())
|
|
display = display_cls(
|
|
stream=RecordingStream(), use_emoji=False, update_interval=0.0
|
|
)
|
|
|
|
display.clear()
|
|
|
|
assert flushed == ["stream"]
|
|
|
|
|
|
class TestEncodingFallback:
|
|
def test_falls_back_when_the_stream_cannot_encode(self, display_cls):
|
|
"""A cp1252-style console must not raise on emoji; it degrades instead."""
|
|
|
|
class AsciiOnly(io.StringIO):
|
|
encoding = "ascii"
|
|
|
|
def write(self, text):
|
|
text.encode("ascii") # raises UnicodeEncodeError on emoji
|
|
return super().write(text)
|
|
|
|
buffer = AsciiOnly()
|
|
display = display_cls(stream=buffer, use_emoji=True, update_interval=0.0)
|
|
|
|
display._safe_write("progress \U0001f504 bar\n")
|
|
|
|
assert "progress" in buffer.getvalue()
|
|
|
|
|
|
if __name__ == "__main__":
|
|
pytest.main([__file__])
|