fix(#4932): wire ordered transparent path to #4927 durable snippet fallback + gate scans to transparent mode
Codex+Opus gate (both reproduced): the new mixed-content ordered render path resolved tool snippets only from the S.messages join (_collectToolResultSnippetsByTid) and never consulted persistedSnippetByTid (S.session.tool_calls) — so on a cold/paginated load where the tool_result join misses (#4927 scenario) the inline card rendered an EMPTY body, AND its tid was added to transparentOrderedToolIds, suppressing the post-loop derived card that WOULD have recovered via #4927. Build a transparentPersistedSnippetByTid map from S.session.tool_calls and thread it as a 5th arg to _transparentOrderedToolCall, falling back in both the live and non-live branches. Also gate the transparent-only scans (S.toolCalls + _collectToolResultSnippetsByTid) behind isTransparentStream() so non-transparent renders skip the O(messages×parts) work (Opus perf finding). Redaction (#4926/#4928) was already safe (shared buildToolCard). + cold-load persisted-fallback regression test.
This commit is contained in:
1 parent
3a406512b0
commit
c4b9edc84e
2 files changed
+65
-9
No files matched your search
+27
-7
@@ -11181,15 +11181,16 @@ function _collectToolResultSnippetsByTid(messages){
|
|||||||
}
|
}
|
||||||
return resultsByTid;
|
return resultsByTid;
|
||||||
}
|
}
|
||||||
function _transparentOrderedToolCall(part, rawIdx, toolCallsByTid, resultsByTid){
|
function _transparentOrderedToolCall(part, rawIdx, toolCallsByTid, resultsByTid, persistedByTid){
|
||||||
const tid=String(part&&part.toolUseId||'').trim();
|
const tid=String(part&&part.toolUseId||'').trim();
|
||||||
const liveTool=tid&&toolCallsByTid&&toolCallsByTid.get(tid);
|
const liveTool=tid&&toolCallsByTid&&toolCallsByTid.get(tid);
|
||||||
if(liveTool){
|
if(liveTool){
|
||||||
const next={...liveTool};
|
const next={...liveTool};
|
||||||
if(resultsByTid&&resultsByTid[tid]){
|
const liveSnip=(resultsByTid&&resultsByTid[tid])||(persistedByTid&&persistedByTid[tid])||'';
|
||||||
|
if(liveSnip){
|
||||||
const patchSnippet=_cliPatchSnippetFromArgs(next.name||part.name||'tool', next.args||part.input||{});
|
const patchSnippet=_cliPatchSnippetFromArgs(next.name||part.name||'tool', next.args||part.input||{});
|
||||||
next.snippet=_cliToolCardSnippet(resultsByTid[tid],patchSnippet);
|
next.snippet=_cliToolCardSnippet(liveSnip,patchSnippet);
|
||||||
next.is_diff=_cliToolCardHasDiffSnippet(resultsByTid[tid],patchSnippet);
|
next.is_diff=_cliToolCardHasDiffSnippet(liveSnip,patchSnippet);
|
||||||
}
|
}
|
||||||
if(next.done===undefined) next.done=true;
|
if(next.done===undefined) next.done=true;
|
||||||
return next;
|
return next;
|
||||||
@@ -11197,7 +11198,7 @@ function _transparentOrderedToolCall(part, rawIdx, toolCallsByTid, resultsByTid)
|
|||||||
const name=part&&part.name||'tool';
|
const name=part&&part.name||'tool';
|
||||||
const args=(part&&part.input&&typeof part.input==='object')?part.input:{};
|
const args=(part&&part.input&&typeof part.input==='object')?part.input:{};
|
||||||
const patchSnippet=_cliPatchSnippetFromArgs(name,args);
|
const patchSnippet=_cliPatchSnippetFromArgs(name,args);
|
||||||
const resultSnippet=(resultsByTid&&tid&&resultsByTid[tid])||'';
|
const resultSnippet=(resultsByTid&&tid&&resultsByTid[tid])||(persistedByTid&&tid&&persistedByTid[tid])||'';
|
||||||
return {
|
return {
|
||||||
name,
|
name,
|
||||||
tid,
|
tid,
|
||||||
@@ -11554,6 +11555,11 @@ function renderMessages(options){
|
|||||||
}
|
}
|
||||||
const transparentOrderedToolIds=new Set();
|
const transparentOrderedToolIds=new Set();
|
||||||
const transparentOrderedToolCallsByTid=new Map();
|
const transparentOrderedToolCallsByTid=new Map();
|
||||||
|
// These scans only feed the transparent-stream ordered render path; skip the
|
||||||
|
// O(messages×parts) work entirely in other modes (Opus perf finding #4932).
|
||||||
|
const _transparentModeActive=(typeof isTransparentStream==='function')&&isTransparentStream();
|
||||||
|
const transparentPersistedSnippetByTid={};
|
||||||
|
if(_transparentModeActive){
|
||||||
if(Array.isArray(S.toolCalls)){
|
if(Array.isArray(S.toolCalls)){
|
||||||
for(const tc of S.toolCalls){
|
for(const tc of S.toolCalls){
|
||||||
if(!tc||typeof tc!=='object') continue;
|
if(!tc||typeof tc!=='object') continue;
|
||||||
@@ -11561,7 +11567,21 @@ function renderMessages(options){
|
|||||||
if(tid&&!transparentOrderedToolCallsByTid.has(tid)) transparentOrderedToolCallsByTid.set(tid,tc);
|
if(tid&&!transparentOrderedToolCallsByTid.has(tid)) transparentOrderedToolCallsByTid.set(tid,tc);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
const transparentToolResultsByTid=_collectToolResultSnippetsByTid(S.messages);
|
// #4927 durable fallback: the ordered path must consult the persisted
|
||||||
|
// session.tool_calls snippet by tid too, or a cold/paginated load where the
|
||||||
|
// S.messages tool_result join misses renders an empty body — and its inline
|
||||||
|
// card then suppresses the post-loop derived card that WOULD have recovered.
|
||||||
|
try{
|
||||||
|
const persisted=(S.session&&Array.isArray(S.session.tool_calls))?S.session.tool_calls:[];
|
||||||
|
persisted.forEach(tc=>{
|
||||||
|
if(!tc||typeof tc!=='object') return;
|
||||||
|
const ptid=tc.tid||tc.id||tc.tool_call_id||tc.call_id||'';
|
||||||
|
const psnip=tc.snippet||tc.result||tc.output||tc.preview||'';
|
||||||
|
if(ptid&&psnip&&!transparentPersistedSnippetByTid[ptid]) transparentPersistedSnippetByTid[ptid]=String(psnip);
|
||||||
|
});
|
||||||
|
}catch(e){}
|
||||||
|
}
|
||||||
|
const transparentToolResultsByTid=_transparentModeActive?_collectToolResultSnippetsByTid(S.messages):{};
|
||||||
// Windowed render loop replaces the legacy full loop:
|
// Windowed render loop replaces the legacy full loop:
|
||||||
// for(let vi=0;vi<visWithIdx.length;vi++)
|
// for(let vi=0;vi<visWithIdx.length;vi++)
|
||||||
for(let vi=0;vi<renderVisWithIdx.length;vi++){
|
for(let vi=0;vi<renderVisWithIdx.length;vi++){
|
||||||
@@ -11771,7 +11791,7 @@ function renderMessages(options){
|
|||||||
orderedTransparentParts.forEach((part, partIdx)=>{
|
orderedTransparentParts.forEach((part, partIdx)=>{
|
||||||
if(!part) return;
|
if(!part) return;
|
||||||
if(part.kind==='tool'){
|
if(part.kind==='tool'){
|
||||||
const toolCall=_transparentOrderedToolCall(part, rawIdx, transparentOrderedToolCallsByTid, transparentToolResultsByTid);
|
const toolCall=_transparentOrderedToolCall(part, rawIdx, transparentOrderedToolCallsByTid, transparentToolResultsByTid, transparentPersistedSnippetByTid);
|
||||||
const toolRow=_decorateTransparentEventRow(buildToolCard(toolCall),{
|
const toolRow=_decorateTransparentEventRow(buildToolCard(toolCall),{
|
||||||
type:'tool',
|
type:'tool',
|
||||||
name:toolCall&&toolCall.name,
|
name:toolCall&&toolCall.name,
|
||||||
|
|||||||
@@ -116,11 +116,47 @@ def test_render_messages_wires_ordered_parts_into_transparent_stream_and_skips_d
|
|||||||
assert "const partDisplayText=_transparentOrderedDisplayText(part.text);" in UI_JS
|
assert "const partDisplayText=_transparentOrderedDisplayText(part.text);" in UI_JS
|
||||||
assert "const partBodyHtml=_getCachedRender(partDisplayText,false);" in UI_JS
|
assert "const partBodyHtml=_getCachedRender(partDisplayText,false);" in UI_JS
|
||||||
assert "const transparentOrderedToolIds=new Set();" in UI_JS
|
assert "const transparentOrderedToolIds=new Set();" in UI_JS
|
||||||
assert "const toolCall=_transparentOrderedToolCall(part, rawIdx, transparentOrderedToolCallsByTid, transparentToolResultsByTid);" in UI_JS
|
assert "const toolCall=_transparentOrderedToolCall(part, rawIdx, transparentOrderedToolCallsByTid, transparentToolResultsByTid, transparentPersistedSnippetByTid);" in UI_JS
|
||||||
assert "if(part.toolUseId) transparentOrderedToolIds.add(part.toolUseId);" in UI_JS
|
assert "if(part.toolUseId) transparentOrderedToolIds.add(part.toolUseId);" in UI_JS
|
||||||
assert "if(tid&&transparentOrderedToolIds.has(tid)) continue;" in UI_JS
|
assert "if(tid&&transparentOrderedToolIds.has(tid)) continue;" in UI_JS
|
||||||
|
|
||||||
|
|
||||||
def test_ordered_tool_cards_refresh_from_settled_results_even_after_live_preview():
|
def test_ordered_tool_cards_refresh_from_settled_results_even_after_live_preview():
|
||||||
assert "if(resultsByTid&&resultsByTid[tid]){" in UI_JS
|
assert "const liveSnip=(resultsByTid&&resultsByTid[tid])||(persistedByTid&&persistedByTid[tid])||'';" in UI_JS
|
||||||
assert "(next.snippet===undefined||next.snippet===null||next.snippet==='')" not in UI_JS
|
assert "(next.snippet===undefined||next.snippet===null||next.snippet==='')" not in UI_JS
|
||||||
|
|
||||||
|
|
||||||
|
def test_ordered_tool_card_falls_back_to_persisted_snippet_on_cold_load():
|
||||||
|
# #4932 gate (#4927 parity): on a cold/paginated load the S.messages
|
||||||
|
# tool_result join misses; the ordered inline card must still recover the
|
||||||
|
# body from the persisted session.tool_calls snippet (persistedByTid),
|
||||||
|
# otherwise its card renders empty AND suppresses the post-loop derived card
|
||||||
|
# that would have recovered it.
|
||||||
|
if NODE is None:
|
||||||
|
import pytest
|
||||||
|
pytest.skip("node not on PATH")
|
||||||
|
driver = r"""
|
||||||
|
const fs=require('fs');
|
||||||
|
const src=fs.readFileSync(process.argv[2],'utf8');
|
||||||
|
function grab(n){const re=new RegExp('function '+n+'\\([^]*?\\n}','m');const m=src.match(re);if(!m)throw new Error('not found '+n);return m[0];}
|
||||||
|
global._cliPatchSnippetFromArgs=()=> '';
|
||||||
|
global._cliToolCardSnippet=(a,b)=> a||b||'';
|
||||||
|
global._cliToolCardHasDiffSnippet=()=> false;
|
||||||
|
global._toolArgsSnapshot=(a)=> a||{};
|
||||||
|
eval(grab('_transparentOrderedToolCall'));
|
||||||
|
const part={kind:'tool', toolUseId:'call_x', name:'terminal', input:{command:'ls'}};
|
||||||
|
// resultsByTid MISSES (cold load), persistedByTid HAS the snippet.
|
||||||
|
const out=_transparentOrderedToolCall(part, 3, new Map(), {}, {call_x:'PERSISTED_OUTPUT_42'});
|
||||||
|
process.stdout.write(JSON.stringify({snippet: out.snippet}));
|
||||||
|
"""
|
||||||
|
import tempfile, os
|
||||||
|
with tempfile.NamedTemporaryFile("w", suffix=".js", delete=False) as fh:
|
||||||
|
fh.write(driver)
|
||||||
|
path = fh.name
|
||||||
|
try:
|
||||||
|
result = subprocess.run([NODE, path, str(UI_JS_PATH)], capture_output=True, text=True, timeout=30)
|
||||||
|
finally:
|
||||||
|
os.unlink(path)
|
||||||
|
assert result.returncode == 0, result.stderr
|
||||||
|
import json as _json
|
||||||
|
assert _json.loads(result.stdout)["snippet"] == "PERSISTED_OUTPUT_42", result.stdout
|
||||||
Reference in new issue
Block a user