From dbcabf475a5d22eb4b2def7b26ad021083d65ecc Mon Sep 17 00:00:00 2001 From: Eva Date: Sun, 31 May 2026 03:57:02 +0700 Subject: [PATCH] fix(viewer): strip internal routing tag from player line in the chronicle (#410) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chronicle showed the player's own action with its internal write-lane routing tag, e.g. `"[do] Without making it obvious…"` (also [say]/[check]/…). The optimistic echo (postMove) already stripped the tag, but the /chat replay of the player's logged line (app.jsx useLiveSession: it.role === "player" -> dialog row) used `it.text` raw. The engine keeps the tag on the logged line for move classification, so once that line round-tripped through /chat it leaked into the transcript (LogEntry's dialog/action branches render entry.text verbatim — only the narration branch sanitizes). Add a shared `window.stripRoutingTag(text)` helper (window-guarded global, like neutralizeMarkup, so the table screen + tests can reach it), apply it on BOTH player-line render paths (the /chat replay and the postMove optimistic echo). Display-only — the write lane keeps the tag for engine routing. Strips the known move verbs (say/do/check/save/continue/attack/cast/use_item/clarify), case-insensitive, leaves untagged text + mid-line brackets untouched, null-safe. Test: viewer/tests/test_player_action_tag_strip.py (pytest; brace-matches the fn out of app.jsx and exercises it under Node, mirroring test_sanitize_narration) — the CI viewer-tests job runs `python -m pytest viewer/tests`. 28 pass locally; 98 green across the related viewer suites. --- viewer/openworlds/app.jsx | 17 ++- viewer/tests/test_player_action_tag_strip.py | 121 +++++++++++++++++++ 2 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 viewer/tests/test_player_action_tag_strip.py diff --git a/viewer/openworlds/app.jsx b/viewer/openworlds/app.jsx index 0d6955c6..33e75873 100644 --- a/viewer/openworlds/app.jsx +++ b/viewer/openworlds/app.jsx @@ -1,5 +1,16 @@ /* App router + tweaks */ +// #410: strip the leading write-lane routing tag ("[do] ", "[say] ", "[check] ", …) from a line +// before it is SHOWN in the chronicle. The tag is internal plumbing the engine uses to classify a +// player move; the player must see their own words, never "[do] opens the door". The write lane +// KEEPS the tag (engine routing) — this is display-only. Shared by every player-line render path: +// the optimistic echo (postMove) AND the /chat replay of the player's logged line. Defined as a +// window-guarded global (like neutralizeMarkup) so screen-table + the pytest suite can reach it. +window.stripRoutingTag = window.stripRoutingTag || function stripRoutingTag(text) { + return String(text == null ? "" : text) + .replace(/^\s*\[(say|do|check|save|continue|attack|cast|use_item|clarify)\]\s*/i, ""); +}; + const TWEAK_DEFAULTS = /*EDITMODE-BEGIN*/{ "palette": "warm", "ornaments": true, @@ -367,7 +378,11 @@ function useLiveSession(state) { .map((it) => { // #274: stamp each beat with the shared monotonic counter at ingest time so it // time-merges correctly against local player echoes (which share the same counter). - if (it.role === "player") return { kind: "dialog", who: "You", text: it.text, at: nextLogSeq() }; + // #410: the engine logs the player's line WITH its routing tag ("[do] …") for move + // classification, and /chat replays it verbatim. Strip the tag for DISPLAY so the + // replayed dialog row shows the player's words, not "[do] …" (matches the optimistic + // echo above, which already strips via the same helper). + if (it.role === "player") return { kind: "dialog", who: "You", text: window.stripRoutingTag(it.text), at: nextLogSeq() }; dmLineArrived = true; // #405: a /chat DM line is the turn-RESOLUTION signal (it clears the pending indicator // below). It is NOT a second narration row when this run is streaming its prose via the diff --git a/viewer/tests/test_player_action_tag_strip.py b/viewer/tests/test_player_action_tag_strip.py new file mode 100644 index 00000000..866f9765 --- /dev/null +++ b/viewer/tests/test_player_action_tag_strip.py @@ -0,0 +1,121 @@ +"""Tests for stripRoutingTag() (#410) — the viewer-side guard that removes the +internal write-lane routing tag ("[do] ", "[say] ", "[check] ", …) from the +player's line BEFORE it is shown in the Chronicle. + +The newbie playtest saw the player's own action rendered as +``"[do] Without making it obvious…"``: the optimistic echo already stripped the +tag, but the /chat replay of the player's *logged* line (which the engine keeps +tagged for move classification) rendered ``it.text`` verbatim, so the tag leaked +once the line round-tripped. Both player-line render paths now share this helper. + +The function lives in app.jsx (browser JS), so — mirroring test_sanitize_narration — +we brace-match its source out of app.jsx and run it under Node. Skipped if Node is +not on PATH. +""" +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +HERE = Path(__file__).resolve().parent +APP_JSX = HERE.parent / "openworlds" / "app.jsx" + + +def _node() -> str: + node = shutil.which("node") + if not node: + pytest.skip("node not on PATH; skipping JS-behavior test") + return node + + +def _run_js(snippet: str) -> str: + proc = subprocess.run([_node(), "-e", snippet], capture_output=True, text=True, timeout=30) + if proc.returncode != 0: + raise AssertionError(f"node failed: {proc.stderr}") + return proc.stdout + + +def _strip_fn_source() -> str: + """Pull `stripRoutingTag`'s source out of app.jsx by brace-matching from + `function stripRoutingTag(` to its closing brace (same approach as + test_sanitize_narration._strip_fn_source).""" + src = APP_JSX.read_text(encoding="utf-8") + marker = "function stripRoutingTag(" + assert marker in src, "stripRoutingTag must be defined in app.jsx (#410)" + start = src.index(marker) + depth = 0 + i = start + while i < len(src): + c = src[i] + if c == "{": + depth += 1 + elif c == "}": + depth -= 1 + if depth == 0: + return src[start : i + 1] + i += 1 + raise AssertionError("could not brace-match stripRoutingTag()") + + +def _strip(text) -> str: + snippet = ( + _strip_fn_source() + + "\nconst __t = " + + json.dumps(text) + + ";\nprocess.stdout.write(String(stripRoutingTag(__t)));\n" + ) + return _run_js(snippet) + + +def test_defined_in_app_jsx_and_registered_on_window(): + # The definition must exist (this is also the latent-bug guard) and be exposed + # on window so the table screen + devtools can reach it. + src = APP_JSX.read_text(encoding="utf-8") + assert "function stripRoutingTag(" in src + # Registered as a window-guarded global (mirrors neutralizeMarkup) so the table + # screen + these tests can reach it. + assert "window.stripRoutingTag" in src + + +def test_strips_the_reported_do_tag(): + # The exact class of string the #324 newbie saw in the chronicle. + assert _strip("[do] Without making it obvious, I scan the room.") == ( + "Without making it obvious, I scan the room." + ) + + +@pytest.mark.parametrize( + "verb", ["say", "do", "check", "save", "continue", "attack", "cast", "use_item", "clarify"] +) +def test_strips_every_write_lane_verb(verb): + # Mirrors viewer/server.py _MOVE_KINDS — every tag the write lane can prepend. + assert _strip(f"[{verb}] hello there") == "hello there" + + +def test_case_insensitive_and_no_space_after_tag(): + assert _strip("[DO]opens the door") == "opens the door" + assert _strip("[Say] wait for the guard") == "wait for the guard" + + +def test_leaves_untagged_text_and_mid_line_brackets_untouched(): + assert _strip("opens the door quietly") == "opens the door quietly" + assert _strip("I shout [for help] down the hall") == "I shout [for help] down the hall" + + +def test_does_not_strip_a_non_routing_bracket_token(): + # Only the known routing verbs are stripped; a stray "[note]" stays as authored. + assert _strip("[note] keep this verbatim") == "[note] keep this verbatim" + + +def test_is_null_and_undefined_safe(): + assert _strip(None) == "" + # JSON has no `undefined`; exercise the JS path directly. + out = _run_js( + _strip_fn_source() + + "\nprocess.stdout.write(String(stripRoutingTag(undefined)));\n" + ) + assert out == ""