-
Notifications
You must be signed in to change notification settings - Fork 35
Version tracker, tracing enhancements and ui changes #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
tysonthomas9
commented
Jul 20, 2025
- Version tracker
- Tracing enhancements
- UI changes
- Tool tests
// found in the LICENSE file. | ||
|
||
export const VERSION_INFO = { | ||
version: '0.1.5', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bump
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive version tracking, enhanced tracing capabilities, and UI improvements to the AI Chat panel. Key additions include automated version checking with update notifications, hierarchical distributed tracing for better observability, and improved user interface elements with accessibility testing infrastructure.
- Version tracking system with automated update notifications and GitHub release integration
- Enhanced hierarchical tracing with improved agent execution observability
- UI improvements including better model selection, updated styling, and agent session visualization
- Comprehensive accessibility testing infrastructure with e2e test resources
Reviewed Changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
test/e2e_non_hosted/ai_chat/accessibility-tree-integration_test.ts | New e2e test for accessibility tree integration testing |
test/e2e_non_hosted/ai_chat/BUILD.gn | Build configuration for ai_chat e2e tests |
test/e2e/resources/ai_chat/*.html | Test HTML resources for accessibility testing scenarios |
front_end/ui/components/markdown_view/markdownView.css | Minor font size adjustment for better readability |
front_end/panels/ai_chat/ui/chatView.css | Extensive UI styling updates including model selector, version banner, and agent timeline styles |
front_end/panels/ai_chat/ui/ChatView.ts | Major UI enhancements with version checking, searchable model selector, and agent session rendering |
front_end/panels/ai_chat/tracing/*.ts | Enhanced tracing system with hierarchical observation support |
front_end/panels/ai_chat/tools/*.ts | Removal of duplicate tracing code from individual tools |
front_end/panels/ai_chat/core/*.ts | Version tracking system and enhanced agent execution with tracing |
Comments suppressed due to low confidence (3)
front_end/panels/ai_chat/ui/ChatView.ts:16
- [nitpick] The import alias 'AgentToolCallMessage' and 'AgentToolResultMessage' are verbose. Consider shorter, clearer aliases like 'AgentToolCall' and 'AgentToolResult'.
import type { AgentSession, AgentMessage, ToolCallMessage as AgentToolCallMessage, ToolResultMessage as AgentToolResultMessage } from '../agent_framework/AgentSessionTypes.js';
front_end/panels/ai_chat/ui/ChatView.ts:294
- [nitpick] The field name '#isVersionBannerDismissed' uses a double negative. Consider renaming to '#showVersionBanner' for clearer logic.
#isVersionBannerDismissed = false;
if (!this.#versionInfo || !this.#versionInfo.isUpdateAvailable || | ||
this.#isVersionBannerDismissed || this.#messages.length > 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complex conditional logic for version banner visibility. Consider extracting to a private method like '#shouldShowVersionBanner()' to improve readability.
if (!this.#versionInfo || !this.#versionInfo.isUpdateAvailable || | |
this.#isVersionBannerDismissed || this.#messages.length > 1) { | |
if (!this.#shouldShowVersionBanner()) { |
Copilot uses AI. Check for mistakes.
private readonly flushInterval = 1000; // 5 seconds | ||
private readonly batchSize = 20; // Reasonable batch size for hierarchical tracing | ||
private readonly flushInterval = 5000; // 5 seconds - less frequent auto-flush | ||
private readonly maxBufferSize = 1000; // Prevent memory exhaustion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Buffer size of 1000 events may be excessive for typical usage. Consider making this configurable or reducing to a more reasonable default like 500.
private readonly maxBufferSize = 1000; // Prevent memory exhaustion | |
private readonly maxBufferSize: number; // Prevent memory exhaustion |
Copilot uses AI. Check for mistakes.
for (const [key, value] of Object.entries(data)) { | ||
// Skip problematic keys that often cause circular references | ||
if (key === 'agentSession' && value && typeof value === 'object') { | ||
// Keep only essential agentSession data | ||
sanitized[key] = { | ||
sessionId: (value as any).sessionId, | ||
agentName: (value as any).agentName, | ||
status: (value as any).status, | ||
messageCount: (value as any).messages?.length || 0, | ||
startTime: (value as any).startTime, | ||
endTime: (value as any).endTime | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coded special handling for 'agentSession' key makes the sanitization logic brittle. Consider using a configuration object or separate sanitization strategies for different data types.
for (const [key, value] of Object.entries(data)) { | |
// Skip problematic keys that often cause circular references | |
if (key === 'agentSession' && value && typeof value === 'object') { | |
// Keep only essential agentSession data | |
sanitized[key] = { | |
sessionId: (value as any).sessionId, | |
agentName: (value as any).agentName, | |
status: (value as any).status, | |
messageCount: (value as any).messages?.length || 0, | |
startTime: (value as any).startTime, | |
endTime: (value as any).endTime | |
}; | |
const sanitizationStrategies: Record<string, (value: any) => any> = { | |
agentSession: (value: any) => ({ | |
sessionId: value.sessionId, | |
agentName: value.agentName, | |
status: value.status, | |
messageCount: value.messages?.length || 0, | |
startTime: value.startTime, | |
endTime: value.endTime, | |
}), | |
}; | |
for (const [key, value] of Object.entries(data)) { | |
// Apply key-specific sanitization strategies if defined | |
if (sanitizationStrategies[key] && value && typeof value === 'object') { | |
sanitized[key] = sanitizationStrategies[key](value); |
Copilot uses AI. Check for mistakes.
|
||
export class VersionChecker { | ||
private static instance: VersionChecker; | ||
private readonly GITHUB_API_URL = 'https://api.github.com/repos/tysonthomas9/browser-operator-devtools-frontend/releases/latest'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coded GitHub repository URL should be configurable or extracted to a configuration file to support different deployments.
Copilot uses AI. Check for mistakes.
|
||
logger.info('Latest version from GitHub:', { tag: releaseData.tag_name, extracted: latestVersion }); | ||
|
||
const comparison = this.compareVersions(this.currentVersion, latestVersion); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Version comparison may fail for non-semantic versions or versions with different formats. Consider using a more robust version comparison library or adding format validation.
Copilot uses AI. Check for mistakes.
// @ts-ignore DevTools context | ||
const utilsModule = window.Root?.Runtime?.cachedResources?.get?.('/front_end/panels/ai_chat/common/utils.js'); | ||
if (!utilsModule?.getAccessibilityTree) { | ||
// Fallback: try dynamic import | ||
// @ts-ignore DevTools context | ||
const module = await import('/front_end/panels/ai_chat/common/utils.js'); | ||
if (module?.getAccessibilityTree) { | ||
// @ts-ignore DevTools context | ||
const SDKModule = await import('/front_end/core/sdk/sdk.js'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple @ts-ignore comments indicate type safety issues. Consider defining proper TypeScript interfaces for the DevTools context objects being accessed.
// @ts-ignore DevTools context | |
const utilsModule = window.Root?.Runtime?.cachedResources?.get?.('/front_end/panels/ai_chat/common/utils.js'); | |
if (!utilsModule?.getAccessibilityTree) { | |
// Fallback: try dynamic import | |
// @ts-ignore DevTools context | |
const module = await import('/front_end/panels/ai_chat/common/utils.js'); | |
if (module?.getAccessibilityTree) { | |
// @ts-ignore DevTools context | |
const SDKModule = await import('/front_end/core/sdk/sdk.js'); | |
const utilsModule = (window.Root?.Runtime?.cachedResources?.get?.('/front_end/panels/ai_chat/common/utils.js') as UtilsModule | undefined); | |
if (!utilsModule?.getAccessibilityTree) { | |
// Fallback: try dynamic import | |
const module = (await import('/front_end/panels/ai_chat/common/utils.js')) as UtilsModule; | |
if (module?.getAccessibilityTree) { | |
const SDKModule = (await import('/front_end/core/sdk/sdk.js')) as SDKModule; |
Copilot uses AI. Check for mistakes.