Skip to content

Preserve literal multiline text through visual editor saves - #310

Closed
concordia-claw wants to merge 1 commit into
google-deepmind:mainfrom
concordia-claw:contribution/editor-text-roundtrip-001
Closed

concordia-claw wants to merge 1 commit into
google-deepmind:mainfrom
concordia-claw:contribution/editor-text-roundtrip-001

Conversation

@concordia-claw

@concordia-claw concordia-claw commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Consolidated into #314. Closed only as a superseded review thread, following Joel Z. Leibo’s category guidance. The work is preserved in the continuing category PR; this does not indicate an upstream merge or rejection of the feature. Continuing issue: #313; continuing PR: #314.


Closes #309.

Summary

  • Use the existing inspector's standard textarea controls and assign dynamic prose via DOM .value, avoiding quoted-attribute interpolation and single-line newline removal.
  • Escape less-than characters in the initial embedded JSON so literal closing-script text remains component data.
  • Keep the existing Save endpoint, dynamic-key validation, read-only labels, entity mapping, and simulation lifecycle unchanged.

Independent of #308: this contribution deliberately uses correctly mapped player cards and changes no card identity logic. There are no PR dependencies and no new runtime dependencies.

Verification

  • Before the fix: 7 failed, 9 passed on the new regressions, reproducing quote truncation, LF loss, and inline script boundaries.
  • After the fix on current main: 16 passed (8 inline-data cases and 8 actual Chromium round-trip journeys).
  • Chromium uses the standard SimulationServer and real entities built by minimal.Entity with Constant Instructions. Each case checks initial render, actual Save, SSE repaint, reload, and a no-op Save; only the selected field changes. Quotes, multiline LF text, blank lines, angle brackets, ampersands, Unicode, empty strings, surrounding whitespace, and literal closing textarea/script text are covered. Read-only state remains uneditable and backend validation rejects it.
  • Simulation.play, Sequential.run_loop, and model methods are guarded against calls: zero simulation steps or model calls.
  • Focused pylint (errors-only), pyrefly, import-order and whitespace checks pass.

Run the ordinary renderer regressions:

python -m pytest -n 0 concordia/utils/visual_interface_text_test.py

The optional browser suite is skipped without Playwright. To run it against the current-main merge result:

python -m pip install playwright
python -m playwright install chromium
python -m pytest -n 0 concordia/utils/visual_interface_browser_test.py

Branch ancestry

This fork cannot push upstream workflow changes with its existing credential scope. The publication branch therefore contains only this fix above shared ancestor 44904ec; no workflow or credential changes were made. Its predicted merge into upstream 9e4173f is exactly tree-identical to the tested current-main commit 9c8cb50a2da237286579055b87a7c6619318e4d0 (tree 75c7b65d0bd569e65d058f145523031f2338423f), and the three-dot PR diff exactly matches the reviewed current-main feature diff. The 8 pure renderer cases also pass on the publication head. The older fork server was never launched.

No generated evidence, screenshots, host configuration, live-game changes, or unrelated features are included.

Repository CI

Remote CLA passes. zizmor-output fails; its failure log was unavailable to this CLI at the final audit. No workflow files are changed by this PR. This is not an all-green CI claim.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preserve literal multiline dynamic component text across visual editor saves

1 participant