Skip to content

Fix queued message placement and improve webview/dev error diagnostics - #47

Merged
tanishqkancharla merged 1 commit into
mainfrom
tk-fix-webview-queue-and-dev-loader
Mar 11, 2026
Merged

Fix queued message placement and improve webview/dev error diagnostics#47
tanishqkancharla merged 1 commit into
mainfrom
tk-fix-webview-queue-and-dev-loader

Conversation

@tanishqkancharla

@tanishqkancharla tanishqkancharla commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • fix message queue rendering so queued messages are separated by timeline position (index-based), not string ID comparisons
  • preserve message time metadata during bootstrap and keep API order instead of re-sorting by ID
  • harden dev webview loader URL handling (OPENCODE_DEV_SERVER_URL) and dynamic port mapping to avoid module MIME/load mismatches
  • improve webview logging so structured objects are serialized (instead of [object Object]) and sendPrompt errors include parsed message/status context

Why

  • users were seeing messages incorrectly dimmed/shown as queued at the bottom due to mixed ID ordering and missing time.completed on bootstrapped messages
  • dev mode webview could fail loading module scripts due to malformed module URLs
  • runtime error diagnostics were hard to debug because object logs were collapsed in console/output

Validation

  • pnpm build
  • pnpm exec vitest run tests/frontend/bootstrap.test.ts

@claude

claude Bot commented Mar 11, 2026

Copy link
Copy Markdown

Claude finished @tanishqkancharla's task in 1m 51s —— View job


PR Review

📋 Detailed Review

Spec Analysis

Based on the PR description and implementation, the goals are:

  • Fix message queue rendering to use index-based position instead of string ID comparison
  • Preserve time metadata during bootstrap and maintain API order (no re-sorting)
  • Harden dev webview loader URL handling with proper URL parsing
  • Improve webview logging to serialize objects properly (instead of [object Object])
  • Add better error context for sendPrompt failures

Changed Files

  • src/OpenCodeViewProvider.ts: Improved dev server URL parsing using new URL() with proper port detection and error handling
  • src/webview/App.tsx: Added getSdkErrorMessage and getResponseStatus helpers for better error diagnostics
  • src/webview/components/MessageList.tsx: Changed queued message detection from string ID comparison to index-based position
  • src/webview/state/bootstrap.ts: Preserved time metadata on messages and removed ID-based re-sorting
  • src/webview/state/eventHandlers.ts: Added logger import (minimal change)
  • src/webview/utils/logger.ts: Added serializeForLog function to handle circular references and Error objects
  • tests/frontend/bootstrap.test.ts: Added test for message time metadata preservation

Simplicity Assessment

  • src/webview/App.tsx:97-136: The getSdkErrorMessage function is verbose with 4 levels of nested error extraction. Consider extracting a helper like extractMessage(obj: unknown): string | null that handles one level, then compose it. However, this defensive extraction is appropriate given SDK error variability - acceptable as-is.

  • src/webview/components/MessageList.tsx:149-176: The index-based approach using pendingAssistantMessageIndex memo is clean and correct. The linear search through messages is acceptable given typical message counts.

  • src/OpenCodeViewProvider.ts:576-607: The URL parsing refactor using new URL() is a clear improvement over string manipulation. The port derivation logic (lines 589-594) is correct and handles edge cases.

  • src/webview/utils/logger.ts:18-48: The serializeForLog function with WeakSet for circular detection is well-implemented. The replacer function properly handles Error objects.

Correctness Assessment

  • src/webview/components/MessageList.tsx:149-158: The pendingAssistantMessageIndex correctly iterates backward to find the last incomplete assistant message using !msg.time?.completed. This properly identifies the currently-generating message.

  • src/webview/state/bootstrap.ts:194-199: The time field is now correctly preserved from msgInfo.time. The message list maintains API order by removing the previous .sort((a, b) => a.id.localeCompare(b.id)) call.

  • src/webview/state/bootstrap.ts:170-201: Messages are processed in API order without re-sorting. This is correct because the server returns messages in chronological order.

  • tests/frontend/bootstrap.test.ts:303-379: The test correctly verifies that API order is preserved (not sorted by ID) and that time metadata is retained. The test uses IDs with different lexicographic vs chronological order to validate this.

Summary

This PR correctly fixes the queued message rendering issue by using index-based position instead of string ID comparison, which was unreliable due to client/server ID format differences. The bootstrap changes properly preserve message metadata and API order. The dev server URL handling is now more robust with proper URL parsing, and the logger improvements will help with debugging object outputs.

Required Actions

None - the implementation achieves all stated goals correctly.

Suggestions

  1. Consider extracting error message extraction (src/webview/App.tsx:97-136): The getSdkErrorMessage function could be slightly simplified by extracting a reusable getNestedMessage(obj) helper, but this is minor and the current implementation is defensive and correct.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@tanishqkancharla
tanishqkancharla merged commit d0d408b into main Mar 11, 2026
4 checks passed
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.

1 participant