ui: give the renderMessages probes the block helper
Four upstream tests read renderMessages out of ui.js and run a slice of it against a fake segment object. Since the message block goes in through _insertSegmentBlock, three of them stopped at "ReferenceError: _insertSegmentBlock is not defined" and the fourth still pinned the old seg.insertAdjacentHTML line, which took twelve of the fifteen test shards red on this branch and nowhere else. test_status_command_card.py defines the helper in its own harness, one line that does what the real helper does without a browser. test_compaction_current_summary_fallback.py and test_anchor_fallback_ownership.py evaluate the real helper from ui.js alongside the functions they already evaluate; their DOM shims return no template content, so it takes its insertAdjacentHTML fallback exactly as before. test_sprint49.py pins the new call, and its point is unchanged: the footer still rides in the same block as the body. A new case in tests/test_issue2051_duplicate_assistant_answer.py pins why the helper exists, which answers a review note: the block is parsed into a template and the nodes are MOVED, so a repeated call adds nothing, and insertAdjacentHTML survives only as the fallback. All three insertions go through it. tests/test_anchor_fallback_ownership.py, tests/test_compaction_current_summary_fallback.py, tests/test_sprint49.py, tests/test_status_command_card.py: 34 passed. The three #2051 files: 12 passed.
This commit is contained in:
1 parent
9d0c629b55
commit
2d48f4c306
5 files changed
+52
-1
No files matched your search
@@ -416,6 +416,10 @@ def test_render_messages_keeps_anchor_owned_turn_out_of_legacy_activity_rebuilds
|
||||
legacy_metadata_source = _function_source(
|
||||
_ui_js(), "_legacySettledFallbackHasToolMetadata"
|
||||
)
|
||||
# #2051: renderMessages() inserts a message block through this helper, so it is
|
||||
# evaluated with it. The shim's createElement() returns no template `content`, so the
|
||||
# helper takes its insertAdjacentHTML fallback here, exactly as before.
|
||||
insert_block_source = _function_source(_ui_js(), "_insertSegmentBlock")
|
||||
script = textwrap.dedent(
|
||||
f"""
|
||||
class FakeClassList {{
|
||||
@@ -702,6 +706,7 @@ def test_render_messages_keeps_anchor_owned_turn_out_of_legacy_activity_rebuilds
|
||||
|
||||
eval({json.dumps(transparent_source)});
|
||||
eval({json.dumps(legacy_metadata_source)});
|
||||
eval({json.dumps(insert_block_source)});
|
||||
eval({json.dumps(render_source)});
|
||||
|
||||
const toolResult = {{ role: 'tool', tool_call_id: 'toolu_1', content: 'tool result' }};
|
||||
|
||||
@@ -50,6 +50,10 @@ _REAL_FUNCTIONS = (
|
||||
"_insertPreservedCompressionTaskFallback",
|
||||
"_pinCompactionCardAtTop",
|
||||
"_pinSettledCompressionReferenceAtTop",
|
||||
# #2051: renderMessages() inserts a message block through this helper, so it has to be
|
||||
# evaluated with it. The shim's createElement() returns no template `content`, so the
|
||||
# helper takes its insertAdjacentHTML fallback here, exactly as before.
|
||||
"_insertSegmentBlock",
|
||||
"renderMessages",
|
||||
)
|
||||
|
||||
|
||||
@@ -336,3 +336,35 @@ def test_both_append_branches_are_gated_by_the_ownership_check():
|
||||
"the segment-swap and whole-turn replace branches must stay unguarded — "
|
||||
"they replace a node instead of adding one and cannot duplicate"
|
||||
)
|
||||
|
||||
|
||||
def test_segment_block_insertion_moves_nodes_instead_of_reparsing_html():
|
||||
"""Wiring: every message-block insertion in renderMessages() goes through the
|
||||
helper, and the helper MOVES parsed nodes instead of re-parsing a string.
|
||||
|
||||
Appending a DocumentFragment moves its children and empties it, so a DOM-API
|
||||
wrapper that executes the call twice appends nothing the second time; an
|
||||
insertAdjacentHTML wrapper that does the same parses and appends a second copy.
|
||||
That difference is the whole fix, so it is pinned here rather than left to the
|
||||
three call sites.
|
||||
"""
|
||||
helper = UI_JS[UI_JS.index("function _insertSegmentBlock("):]
|
||||
helper = helper[:helper.index("\nfunction renderMessages")]
|
||||
assert "createElement('template')" in helper, (
|
||||
"the block must be parsed into a <template>, not inserted as a string"
|
||||
)
|
||||
assert "seg.appendChild(tpl.content)" in helper, (
|
||||
"the template's content must be MOVED into the segment — that is what makes "
|
||||
"a repeated call a no-op"
|
||||
)
|
||||
assert helper.count("insertAdjacentHTML") == 1, (
|
||||
"insertAdjacentHTML may survive only as the fallback for an environment "
|
||||
"without <template>"
|
||||
)
|
||||
render = UI_JS[UI_JS.index("function renderMessages(options){"):]
|
||||
assert render.count("_insertSegmentBlock(") == 3, (
|
||||
"all three assistant-segment block insertions must go through the helper"
|
||||
)
|
||||
assert "seg.insertAdjacentHTML('beforeend', `${filesHtml}" not in render, (
|
||||
"an assistant segment's block must not be built from an HTML string again"
|
||||
)
|
||||
@@ -38,7 +38,10 @@ def test_footer_timestamp_uses_richer_format_for_older_messages():
|
||||
|
||||
def test_timestamp_footer_stays_on_visible_response_segments():
|
||||
assert "if(hasVisibleBody){" in UI_JS
|
||||
assert 'seg.insertAdjacentHTML(\'beforeend\', `${filesHtml}<div class="msg-body">${bodyHtml}</div>${footHtml}`);' in UI_JS, (
|
||||
# #2051: the block goes in through _insertSegmentBlock() rather than
|
||||
# seg.insertAdjacentHTML(), so a wrapped DOM API cannot render it twice. The footer
|
||||
# still rides in the same block, which is what this test is about.
|
||||
assert '_insertSegmentBlock(seg, `${filesHtml}<div class="msg-body">${bodyHtml}</div>${footHtml}`);' in UI_JS, (
|
||||
"Footer timestamp should stay attached to visible response segments"
|
||||
)
|
||||
assert "assistantThinking.set(rawIdx, thinkingText);" in UI_JS, (
|
||||
|
||||
@@ -87,6 +87,13 @@ function makeSeg() {{
|
||||
}};
|
||||
}}
|
||||
|
||||
// #2051: renderMessages() inserts a message block through this helper so a wrapped
|
||||
// DOM API cannot render it twice. Outside a browser the helper's own fallback is
|
||||
// insertAdjacentHTML, which is exactly what it does here.
|
||||
function _insertSegmentBlock(seg, html) {{
|
||||
seg.insertAdjacentHTML('beforeend', html);
|
||||
}}
|
||||
|
||||
const ordinaryBlock = extractBlock(
|
||||
"const hasVisibleBody=!!(String(content||'').trim()||filesHtml||recoveryHtml);",
|
||||
"_assistantTurnBlocks(currentAssistantTurn).appendChild(seg);"
|
||||
|
||||
Reference in new issue
Block a user