Repository navigation
Add pluggable TUI renderer backends (mattervis.native-v1 default) - #241
Conversation
Keep compose_frame as the built-in mattervis.native-v1 backend and send both static and session frames through one display request. External backends are selected by entry point and fail closed, without changing default JSONL fingerprints. Co-authored-by: Ming-Yu Guo <guomy26@mail2.sysu.edu.cn>
Resolve the CLI renderer in the terminal CLI helper and attach it to the controller after construction so cli.py and the view controller stay within their line caps. Co-authored-by: Ming-Yu Guo <guomy26@mail2.sysu.edu.cn>
SchrodingersCattt
left a comment
There was a problem hiding this comment.
Review — needs-changes (CI not green; do not merge)
Head 50a93099. Author PR → COMMENT only (cannot self-APPROVE).
CI (9 checks)
| Check | Result |
|---|---|
| Fast 3.12 (min / latest) | success |
| Build / Minimal smoke / Benchmark | success |
| Full tests Python 3.12 | success |
| Full tests Python 3.11 | failure |
| Full tests Python 3.10 | failure |
| Camera WebGL regression | still in progress at review time |
Both Full 3.10 and 3.11: 1 failed, 1981 passed, 49 skipped.
Blocker
tests/tui/test_tui_renderer_backend.py::test_frozen_water_and_episode
Exact string compare of episode_ascii7_80x24.jsonl fails on 3.10/3.11. Diff is not picture bytes (frame.sha256 / screen text match the golden). It is viewport float ULP in the JSONL observation:
- golden (likely frozen on 3.12):
y_max/y_min…1549 - 3.10/3.11: same values …
15488
That shifts state_fingerprint and fails assert episode == golden.
This is a real cross-version stability bug in the acceptance golden, not a flake. Default native picture identity still looks fine; the frozen episode JSONL is over-specified.
Concrete fix list (for cloud agent)
- Stop byte-comparing the full episode JSONL across CPython versions in CI.
- Keep strong checks that already match #240:
- frozen frame
.txt/index.jsonsha256 - within-run: orbit changes fingerprint; reset restores fingerprint +
frame.sha256 - legacy observation keys; no
renderer/observation_profileon default - display toggles after
set_display
- frozen frame
- If an episode golden is kept: compare with a float-tolerant JSON walk (or round viewport scale/bounds before fingerprint materialization), and document that picture bytes are the byte-identity contract, not every float bit in viewport.
- Re-run Full on 3.10 and 3.11 (and wait for WebGL) before asking for re-review / merge.
#240 acceptance map (code review; blocked by CI)
| Criterion | Assessment |
|---|---|
Native default byte-identical / state_fingerprint unchanged |
Frames + default session fingerprint logic look correct; episode golden over-asserts float bits |
| Single dispatch static + active | Yes — both call render_picture |
| Entry points; fail-closed; no silent native fallback | Yes — reserved mattervis.native-v1, pin/API/malformed/crash covered |
No CrystalIR/chemistry/gold on RenderRequest; model profile opt-in |
Yes — assert_public_payload, canaries, observation_profile="model" |
External package separate (examples/mattervis_mark_renderer) |
Yes — own wheel + entry point |
| Docs | docs/agents/tui_renderer_api.md + CLI/dev notes |
| Tests: golden / fail-closed / canary / polar suite retained | Present; episode golden fails on 3.10/3.11 |
Nits (non-blocking)
resolve_renderer_bindingassignsstored_backendtwice (redundant, harmless).- Plugin
RuntimeErrorpropagates raw (still fail-closed; no native substitute).
Verdict: needs-changes. Fix the episode golden / comparison for 3.10+3.11, get Full green on those, then re-request review. Will not squash-merge until CI is 9/9 success.
Picture text and frame.sha256 stay exact. Viewport floats may differ by one ULP, which changes state_fingerprint, so the episode trace is checked with a float tolerance and within-run fingerprint equality. Co-authored-by: Ming-Yu Guo <guomy26@mail2.sysu.edu.cn>
|
Ready for re-review. The 3.10/3.11 failure was the frozen episode JSONL, not the picture. The episode compare now keeps picture bytes and CI on |
SchrodingersCattt
left a comment
There was a problem hiding this comment.
Re-review — approve-ready (author PR → COMMENT only)
Head 1c89bcc. Prior blocker (Full 3.10/3.11 test_frozen_water_and_episode) is fixed.
CI on this head (9/9 success)
| Check | Result |
|---|---|
| Fast 3.12 (min / latest) | success |
| Build / Minimal smoke / Benchmark | success |
| Full tests Python 3.10 / 3.11 / 3.12 | success |
| Camera WebGL regression | success |
Fix check
_assert_episode_matches_golden + _assert_json_close: picture / frame.sha256 stay exact; viewport floats tolerate ULP; state_fingerprint not cross-CPython byte-compared; within-run orbit≠reset==initial fingerprint still asserted. Hash algorithm unchanged (no pre-hash rounding). Unit test_episode_golden_tolerates_viewport_ulp_not_picture_bytes covers the contract.
#240 acceptance (unchanged from prior map; now unblocked by CI)
Native default frames + single render_picture dispatch, fail-closed entry points, chemistry-free RenderRequest, model profile opt-in, separate marks example package, docs, polar suite retained — all still look good.
Verdict: merge-ready. Squashing next (Closes #240).
Closes #240
Summary
MatterVis now has one TUI frame-renderer contract. Static CLI frames and live session frames both go through
render_picture. The built-in backend id ismattervis.native-v1. It still callscompose_framewith the host crystal and camera, so the default picture stays byte-for-byte on the frozen fixtures.state_fingerprintdoes not include renderer identity, and default observations do not grow arendererfield.An operator selects another backend with
TerminalSession(..., renderer=...)ormat-vis tui --renderer ID --renderer-config FILE. Backends are installed wheels in themattervis.tui.renderersentry-point group. A missing id, wrong API, bad pin, unsupported option, malformed frame, or plugin exception fails closed. Dispatch does not draw the native picture instead. Renderer choice is episode configuration, not aset_displayaction.External
rendermethods receiveRenderRequestonly. The host buildsRenderedFrameand the hashes.CrystalIR, MolCrysKit chemistry, gold, and scorer fields are not on that request.observation_profile="model"is opt-in and strips the source title, chemistry warnings, and formula/composition scope fields.examples/mattervis_mark_rendereris a separate package (example.marks-v1). It draws depth digits and bond dots and never element symbols. It is not copied intomat_viewer.Picture bytes are the cross-CPython byte-identity contract for the default native backend. Viewport floats can differ by one ULP across interpreters, which changes
state_fingerprint. The frozen episode is compared with that tolerance. Within one process, reset still restores the fingerprint and a camera change still replaces it. Viewport values are not rounded before hashing, so default fingerprints stay on the historical algorithm.Acceptance
observe → orbit → set_display → resetepisode keeps exact frame text andframe.sha256, legacy keys, and within-run fingerprint equality. The full JSONL document is not required to be byte-identical across CPython versions.state_fingerprintstay aligned with native.provided/inferred/unspecified) are preserved.provided_onlydisables inferred edges without mutating the source graph.replay_manifest()plus, on non-default episodes,input_digest,view_digest, and unpaddedcontent_sha256.docs/agents/tui_renderer_api.md. CLI notes and developer dispatch notes are updated. Polar-orbit tests stay in the suite.What was and was not validated
Validated here: default compositor picture byte identity, fail-closed selection, charset bounds, camera and action stability across backends, and canaries for hidden chemistry fields.
Not scientifically validated: an external renderer can still infer chemistry from coordinates. This change does not validate a benchmark, a scorer, a task split, or a human reading of either picture. A Python
Protocolis not a sandbox; untrusted renderers need a separate process. This does not implement research harness issue 17 and does not close #163. No subprocess renderer is shipped.Test plan
python -m pytest tests/tui/test_tui_renderer_backend.py tests/tui/test_tui_session.py tests/tui/test_tui_polar_escape.pypython -m pytest tests/ -n 4 --dist loadfile -m "not browser"on Python 3.121c89bcc