fix(#3971): move show-thinking gate to the real Worklog render paths (Opus catch)
Opus final gate caught that the staged re-implementation placed the window._showThinking===false gate in _worklogReasonNodeFromText, which has ZERO callers — so live/settled reasoning rows (built by _syncWorklogReasonFromAnchor and _appendWorklogReason) were never actually hidden, and removeThinking swept data-worklog-reason-source=reasoning which the real rows (data-worklog-anchor-reason=1) don't carry. The string-match tests passed against the dead function = green-suite blind spot. Fix: - Gate _syncWorklogReasonFromAnchor (live + settled): when _showThinking===false, remove any existing reasoning row and bail before building. - Gate _appendWorklogReason (settled rebuild): return null when _showThinking===false. - removeThinking + the settled-rebuild cleanup now sweep BOTH data-worklog-anchor-reason=1 (the real attribute) and the legacy data-worklog-reason-source=reasoning. - Re-anchor the tests to assert against the real render functions + the correct attribute.
This commit is contained in:
nesquena-hermes
17
static/ui.js
17
static/ui.js
@@ -7275,10 +7275,19 @@ function _syncWorklogReasonFromAnchor(group, anchor, displayTextOverride){
|
||||
const list=_toolWorklogListEl(group);
|
||||
if(!group||!list) return;
|
||||
const anchorKey=_worklogReasonAnchorKey(anchor);
|
||||
const selector=anchorKey?`:scope > .wl-reason[data-worklog-anchor-key="${CSS.escape(anchorKey)}"]`:':scope > .wl-reason[data-worklog-anchor-reason="1"]';
|
||||
// When reasoning/thinking display is turned off (#3903), do not render Worklog
|
||||
// reasoning rows on the live OR settled path — remove any existing one and bail
|
||||
// before building. (The gate must live here, in the actual render path, not in
|
||||
// the unused _worklogReasonNodeFromText helper.)
|
||||
if(window._showThinking===false){
|
||||
const existing=list.querySelector(selector);
|
||||
if(existing) existing.remove();
|
||||
return;
|
||||
}
|
||||
const html=arguments.length>2
|
||||
? _worklogReasonHtmlFromAnchor(anchor, displayTextOverride)
|
||||
: _worklogReasonHtmlFromAnchor(anchor);
|
||||
const selector=anchorKey?`:scope > .wl-reason[data-worklog-anchor-key="${CSS.escape(anchorKey)}"]`:':scope > .wl-reason[data-worklog-anchor-reason="1"]';
|
||||
let reason=list.querySelector(selector);
|
||||
if(!html){
|
||||
if(reason) reason.remove();
|
||||
@@ -7344,6 +7353,8 @@ function _migrateLegacyLiveActivityGroupsToWorklog(blocks, worklog){
|
||||
}
|
||||
function _appendWorklogReason(list, anchor){
|
||||
if(!list) return null;
|
||||
// Reasoning display off (#3903): never append a Worklog reasoning row.
|
||||
if(window._showThinking===false) return null;
|
||||
const html=_worklogReasonHtmlFromAnchor(anchor);
|
||||
if(!html) return null;
|
||||
const reason=document.createElement('div');
|
||||
@@ -9288,7 +9299,7 @@ function renderMessages(options){
|
||||
// regression vs master; same content-loss-on-switch class as #3668). The
|
||||
// `:not([data-live-thinking="1"])` / live-card guards below keep the active
|
||||
// turn's own live nodes from being double-built.
|
||||
inner.querySelectorAll('.tool-worklog-group:not([data-compression-card]),.tool-call-group:not([data-compression-card]),.tool-card-row:not([data-compression-card]),.agent-activity-thinking:not([data-live-thinking="1"]):not([data-event-type="thinking"]),.wl-reason[data-worklog-reason-source="reasoning"]').forEach(el=>el.remove());
|
||||
inner.querySelectorAll('.tool-worklog-group:not([data-compression-card]),.tool-call-group:not([data-compression-card]),.tool-card-row:not([data-compression-card]),.agent-activity-thinking:not([data-live-thinking="1"]):not([data-event-type="thinking"]),.wl-reason[data-worklog-anchor-reason="1"],.wl-reason[data-worklog-reason-source="reasoning"]').forEach(el=>el.remove());
|
||||
const byActivity = new Map();
|
||||
const assistantIdxs=[...assistantSegments.keys()].sort((a,b)=>a-b);
|
||||
const _assistantAnchorForActivity=(aIdx,segmentSeq,burstId)=>{
|
||||
@@ -11478,7 +11489,7 @@ function removeThinking(){
|
||||
const turn=$('liveAssistantTurn');
|
||||
const blocks=_assistantTurnBlocks(turn);
|
||||
if(blocks) blocks.querySelectorAll('.agent-activity-thinking').forEach(el=>el.remove());
|
||||
if(blocks) blocks.querySelectorAll('.wl-reason[data-worklog-reason-source="reasoning"]').forEach(el=>el.remove());
|
||||
if(blocks) blocks.querySelectorAll('.wl-reason[data-worklog-anchor-reason="1"],.wl-reason[data-worklog-reason-source="reasoning"]').forEach(el=>el.remove());
|
||||
if(blocks) blocks.querySelectorAll('.live-worklog[data-live-worklog-shell="1"],.tool-worklog-group[data-live-tool-call-group="1"],.tool-call-group[data-live-tool-call-group="1"],.tool-call-group[data-agent-activity-group="1"]').forEach(group=>{
|
||||
_syncToolCallGroupSummary(group);
|
||||
if(!group.querySelector('.tool-card-row,.agent-activity-thinking,.wl-reason')){
|
||||
|
||||
@@ -99,9 +99,17 @@ class TestUiJsThinkingGate:
|
||||
|
||||
def test_worklog_reasoning_rows_are_gated_by_show_thinking(self):
|
||||
src = read('static/ui.js')
|
||||
node_fn = function_body(src, "_worklogReasonNodeFromText")
|
||||
assert 'window._showThinking===false' in node_fn and 'return null' in node_fn, (
|
||||
"reasoning-source Worklog rows must not be created when thinking is hidden"
|
||||
# The gate must sit in the ACTUAL render paths that build Worklog reasoning
|
||||
# rows — _syncWorklogReasonFromAnchor (live + settled) and _appendWorklogReason
|
||||
# (settled rebuild) — not in the unused _worklogReasonNodeFromText helper.
|
||||
sync_fn = function_body(src, "_syncWorklogReasonFromAnchor")
|
||||
assert 'window._showThinking===false' in sync_fn and 'return' in sync_fn, (
|
||||
"_syncWorklogReasonFromAnchor must bail (and remove any existing row) "
|
||||
"when thinking display is off"
|
||||
)
|
||||
append_fn = function_body(src, "_appendWorklogReason")
|
||||
assert 'window._showThinking===false' in append_fn and 'return null' in append_fn, (
|
||||
"_appendWorklogReason must not build a reasoning row when thinking is hidden"
|
||||
)
|
||||
|
||||
def test_show_thinking_gate_does_not_hide_worklog_anchor_text(self):
|
||||
@@ -115,8 +123,12 @@ class TestUiJsThinkingGate:
|
||||
def test_remove_thinking_prunes_reasoning_rows_but_preserves_tool_or_anchor_rows(self):
|
||||
src = read('static/ui.js')
|
||||
fn = function_body(src, "removeThinking")
|
||||
assert '.wl-reason[data-worklog-reason-source="reasoning"]' in fn, (
|
||||
"removeThinking must sweep already-rendered reasoning Worklog rows"
|
||||
# The live/settled reasoning rows are tagged data-worklog-anchor-reason="1"
|
||||
# (by _syncWorklogReasonFromAnchor / _appendWorklogReason); the sweep MUST
|
||||
# target that attribute, not only the legacy data-worklog-reason-source.
|
||||
assert '.wl-reason[data-worklog-anchor-reason="1"]' in fn, (
|
||||
"removeThinking must sweep the actually-rendered reasoning Worklog rows "
|
||||
'(data-worklog-anchor-reason="1")'
|
||||
)
|
||||
assert '.tool-card-row,.agent-activity-thinking,.wl-reason' in fn, (
|
||||
"empty-group cleanup must preserve groups that still contain tool cards "
|
||||
|
||||
Reference in New Issue
Block a user