Skip to content

Commit ebac36e

Browse files
cameronrsudo-tee
authored andcommitted
fix(renderer): parts occasionally not rendering
This was a subtle one and it only shows up when we're not collapsing events so it wasn't caught by the unit tests. I've since modified the unit tests to also run the replays without collapsing to catch bugs like this in the future. If the first version of a part didn't result in something being rendered, future updates to that part would also not render anything. This was particularly likely to happen with tools as the first version of the part is often `step-start`. Because we didn't think this was a new part, we called `_replace_part_in_buffer` but that exits quickly because the part hadn't been rendered anywhere. While we could try to call `_insert_part_to_buffer` in `_replace_part_in_buffer` if the part hadn't been rendered, that would miss some subtle cases around error handling so the correct fix is consider the part new in `on_part_updated` if we haven't rendered it before.
1 parent 313acb8 commit ebac36e

File tree

3 files changed

+37
-17
lines changed

3 files changed

+37
-17
lines changed

lua/opencode/ui/render_state.lua

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,16 +243,18 @@ end
243243

244244
---Update part data reference
245245
---@param part_ref OpencodeMessagePart New part reference (must include id)
246+
---@return RenderedPart? part The rendered part
246247
function RenderState:update_part_data(part_ref)
247248
if not part_ref or not part_ref.id then
248249
return
249250
end
250-
local part_data = self._parts[part_ref.id]
251-
if not part_data then
251+
local rendered_part = self._parts[part_ref.id]
252+
if not rendered_part then
252253
return
253254
end
254255

255-
part_data.part = part_ref
256+
rendered_part.part = part_ref
257+
return rendered_part
256258
end
257259

258260
---Helper to update action line numbers

lua/opencode/ui/renderer.lua

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ end
280280
function M._replace_part_in_buffer(part_id, formatted_data)
281281
local cached = M._render_state:get_part(part_id)
282282
if not cached or not cached.line_start or not cached.line_end then
283+
-- return M._insert_part_to_buffer(part_id, formatted_data)
283284
return false
284285
end
285286

@@ -536,7 +537,13 @@ function M.on_part_updated(properties, revert_index)
536537
if is_new_part then
537538
M._render_state:set_part(part)
538539
else
539-
M._render_state:update_part_data(part)
540+
local rendered_part = M._render_state:update_part_data(part)
541+
-- NOTE: This isn't the first time we've seen the part but we haven't rendered it previously
542+
-- so try and render it this time by setting is_new_part = true (otherwise we'd call
543+
-- _replace_message_in_buffer and it wouldn't do anything because the part hasn't been rendered)
544+
if not rendered_part or (not rendered_part.line_start and not rendered_part.line_end) then
545+
is_new_part = true
546+
end
540547
end
541548

542549
local formatted = formatter.format_part(part, message, is_last_part)

tests/unit/renderer_spec.lua

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ local ui = require('opencode.ui.ui')
33
local helpers = require('tests.helpers')
44
local output_window = require('opencode.ui.output_window')
55
local assert = require('luassert')
6+
local config = require('opencode.config')
67

78
local function assert_output_matches(expected, actual, name)
89
local normalized_extmarks = helpers.normalize_namespace_ids(actual.extmarks)
@@ -120,19 +121,29 @@ describe('renderer', function()
120121
local expected_path = 'tests/data/' .. name .. '.expected.json'
121122

122123
if vim.fn.filereadable(expected_path) == 1 then
123-
it('replays ' .. name .. ' correctly (event-by-event)', function()
124-
local events = helpers.load_test_data(filepath)
125-
state.active_session = helpers.get_session_from_events(events)
126-
local expected = helpers.load_test_data(expected_path)
127-
128-
helpers.replay_events(events)
129-
vim.wait(1000, function()
130-
return vim.tbl_isempty(state.event_manager.throttling_emitter.queue)
131-
end)
132-
133-
local actual = helpers.capture_output(state.windows and state.windows.output_buf, output_window.namespace)
134-
assert_output_matches(expected, actual, name)
135-
end)
124+
for i = 1, 2 do
125+
config.ui.output.rendering.event_collapsing = i == 1 and true or false
126+
it(
127+
'replays '
128+
.. name
129+
.. ' correctly (event-by-event, '
130+
.. (config.ui.output.rendering.event_collapsing and 'collapsing' or 'no collapsing')
131+
.. ')',
132+
function()
133+
local events = helpers.load_test_data(filepath)
134+
state.active_session = helpers.get_session_from_events(events)
135+
local expected = helpers.load_test_data(expected_path)
136+
137+
helpers.replay_events(events)
138+
vim.wait(1000, function()
139+
return vim.tbl_isempty(state.event_manager.throttling_emitter.queue)
140+
end)
141+
142+
local actual = helpers.capture_output(state.windows and state.windows.output_buf, output_window.namespace)
143+
assert_output_matches(expected, actual, name)
144+
end
145+
)
146+
end
136147

137148
if not vim.tbl_contains(skip_full_session, name) then
138149
it('replays ' .. name .. ' correctly (session)', function()

0 commit comments

Comments
 (0)