Dev #9
Dev #9
Conversation
chore: update dependencies to latest versions docs: update workflow integration documentation fix(af.ts): improve type safety in request info parsing style: organize imports and improve code formatting build: update package.json dependencies and resolutions
- Add dracula theme with new color tokens - Implement theme cycling in command handler - Add score display to suggestions - Improve status line and header messaging - Add syntax highlighting for code blocks - Implement adaptive text input component - Add search and tabs to settings menu - Improve session list with search and actions - Add tests for new components and features - Update documentation for new components
- Add new messages indicator when not at bottom - Improve auto-scroll logic with initial load handling - Add smooth scrolling animation and enable/disable controls - Include UI elements for better user interaction
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbitRelease Notes v0.1.5
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRefactors the app into hook/service modules, adds OpenAPI/Kiota specs and Agent Framework bridge env vars, introduces new UI components (AdaptiveTextInput, PromptOverlay, StatusLine, etc.) and Dracula theme, updates CI/package management for Bun, enhances auto-scroll UI, streaming service, AF parsing, tests, and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as App (index.tsx)
participant Hooks as Hooks (useInputMode/useStreaming/useAppState)
participant Service as Services (commandService/streamingService)
participant API as OpenAI / AF bridge
User->>UI: type / submit
UI->>Hooks: detect mode (useInputMode)
Hooks-->>UI: suggestions, inputMode
alt command mode
UI->>Service: executeCommand(cmd)
Service-->>UI: CommandResult
else streaming mode
UI->>Hooks: useStreaming.streamResponse(...)
Hooks->>Service: streamResponseFromOpenAI / startWorkflow
Service->>API: POST /responses or AF bridge
API-->>Service: SSE chunks / deltas
Service->>Hooks: onDelta/onError/onDone
Hooks-->>UI: update assistant message
end
UI->>Hooks: setMessages / persist
Hooks-->>UI: re-render
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (49)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
- Add debounce utility and apply to file I/O operations to reduce disk writes - Optimize message updates with direct array indexing instead of O(n) map - Extract SSE line processing into reusable function to eliminate duplicate code - Add comprehensive tests for debounce utility - Document performance optimizations in PERFORMANCE_OPTIMIZATIONS.md
There was a problem hiding this comment.
Pull request overview
This PR refactors the qlaw-cli codebase by extracting logic into services and hooks, adds UI improvements (auto-scroll with animation, new messages indicator), introduces a Dracula theme with cycling support, and updates dependencies. The changes improve code organization and maintainability while adding new features.
Key Changes:
- Extracted streaming, command execution, and UI state management into separate services and hooks
- Added enhanced auto-scroll functionality with smooth animations and user controls
- Introduced Dracula theme and theme cycling (dark → light → dracula)
Reviewed changes
Copilot reviewed 48 out of 52 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| web-ui/af-workflows-ui.ts | Enhanced auto-scroll with animation, UI controls, and destroy method |
| web-ui/af-workflows-ui.css | Added styles for new messages indicator and scroll button |
| tests/* | New test files for scroll, theme, streaming, command, and adaptive input |
| src/services/* | Extracted streamingService and commandService from index.tsx |
| src/hooks/* | New hooks for streaming, settings, sessions, keyboard, input mode, app state |
| src/components/* | New StatusLine, PromptOverlay, AdaptiveTextInput components |
| src/index.tsx | Refactored to use new hooks and services (498 lines → 117 lines) |
| src/themes.ts | Added DRACULA theme |
| src/commandHandlers.ts | Theme cycling logic updated |
| src/api.ts | Added getOpenAIEnv helper |
| src/types.ts | Added "dracula" to ThemeName, score to UISuggestion |
| package.json | Version bump to 0.1.5, OpenTUI updates, removed applicationinsights |
| bun.lock | Updated dependencies |
| docs/* | New release notes, architecture docs, AdaptiveTextInput docs |
| openapi.json | New OpenAPI specification file |
| .kiota/* | New Kiota configuration files |
| .gitignore | Added bridge.db |
src/hooks/useSettings.ts
Outdated
| { | ||
| id: "theme", | ||
| label: "Theme", | ||
| value: settings.theme === "dark" ? "Dark" : "Light", |
There was a problem hiding this comment.
The theme value display logic doesn't account for the new "dracula" theme. When theme is "dracula", it will display "Light" which is incorrect. Update to handle all three theme values: settings.theme === "dark" ? "Dark" : settings.theme === "light" ? "Light" : "Dracula".
src/components/StatusLine.tsx
Outdated
| ? settings.afBridgeBaseUrl | ||
| ? "Workflow • AF bridge ready" | ||
| : "Workflow • AF bridge not set" | ||
| : "Standard • Set OPENAI_API_KEY to enable streaming"; |
There was a problem hiding this comment.
The error message "Standard • Set OPENAI_API_KEY to enable streaming" is misleading. The streaming functionality depends on OPENAI_BASE_URL, OPENAI_API_KEY, and OPENAI_MODEL (all three), not just the API key. Update to: "Standard • Configure OpenAI settings to enable streaming" or similar.
src/index.tsx
Outdated
| import { SessionList } from "./components/SessionList.tsx"; | ||
| import { SettingsMenu } from "./components/SettingsMenu.tsx"; | ||
| import { Header } from "./components/Header.tsx"; | ||
| // Header component not used |
There was a problem hiding this comment.
Dead code comment indicates the Header import is unused but still present. Remove the commented-out import statement entirely: import { Header } from "./components/Header.tsx"; should be deleted.
|
|
||
| // Streaming spinner animation | ||
| useEffect(() => { | ||
| let intervalId: ReturnType<typeof setInterval> | undefined; |
There was a problem hiding this comment.
[nitpick] The intervalId type should be Timer | undefined in Bun environments for consistency with Bun's runtime types, though ReturnType<typeof setInterval> works. Consider using the more explicit Timer type from Bun's globals for clarity.
| const start = Date.now(); | ||
| const step = () => { | ||
| const t = Math.min(1, (Date.now() - start) / durationMs); | ||
| const eased = t < 0.5 ? 2 * t * t : -1 + (4 - 2 * t) * t; |
There was a problem hiding this comment.
The easing function used here (ease-in-out quad) lacks documentation. Add a comment explaining the easing function: // Ease-in-out quadratic easing for smooth animation.
src/af.ts
Outdated
| ? ri.data.conversation.filter((m): m is ConversationMessage => | ||
| typeof m === 'object' && m !== null && | ||
| typeof m.role === 'string' && | ||
| typeof m.text === 'string' | ||
| typeof (m as ConversationMessage).role === 'string' && | ||
| typeof (m as ConversationMessage).text === 'string' | ||
| ) |
There was a problem hiding this comment.
[nitpick] The type guard uses repeated type assertions (m as ConversationMessage) which could be simplified. Consider using a helper function or destructuring: const { role, text } = m as ConversationMessage; return typeof role === 'string' && typeof text === 'string'.
src/components/AdaptiveTextInput.tsx
Outdated
| @@ -0,0 +1,160 @@ | |||
| import { TextAttributes, type TextareaRenderable } from "@opentui/core"; | |||
| import { createElement, useCallback, useEffect, useMemo, useRef, useState } from "react"; | |||
There was a problem hiding this comment.
Unused import createElement.
| import { createElement, useCallback, useEffect, useMemo, useRef, useState } from "react"; | |
| import { useCallback, useEffect, useMemo, useRef, useState } from "react"; |
src/hooks/useInputMode.ts
Outdated
| * Handles command/mention detection and suggestion generation | ||
| */ | ||
|
|
||
| import { useState, useEffect, useMemo } from "react"; |
There was a problem hiding this comment.
Unused import useMemo.
tests/theme-cycle.test.ts
Outdated
| @@ -0,0 +1,28 @@ | |||
| import { expect, test, describe } from "bun:test"; | |||
| import { getTheme, DARK, LIGHT, DRACULA } from "../src/themes"; | |||
There was a problem hiding this comment.
Unused imports DARK, LIGHT.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
…mports Co-authored-by: Zochory <60674042+Zochory@users.noreply.github.com>
Address code review feedback: fix theme display, error messages, and unused imports
src/hooks/useAppState.ts
Outdated
| // Save current session when messages change | ||
| useEffect(() => { | ||
| if (currentSessionId && messages.length > 0) { | ||
| setSessions((prev) => | ||
| prev.map((s) => | ||
| s.id === currentSessionId | ||
| ? { ...s, messages, updatedAt: new Date() } | ||
| : s | ||
| ) | ||
| ); | ||
| } | ||
| }, [messages, currentSessionId, setSessions]); |
There was a problem hiding this comment.
The setSessions dependency in the useEffect hook on line 78 is unnecessary and could cause infinite re-renders. The setSessions function from useState is stable and doesn't need to be in the dependency array. This violates the rules of hooks dependency optimization.
src/hooks/useSettings.ts
Outdated
| { | ||
| id: "theme", | ||
| label: "Theme", | ||
| value: settings.theme === "dark" ? "Dark" : settings.theme === "light" ? "Light" : "Dracula", | ||
| description: "Toggle dark/light palette", |
There was a problem hiding this comment.
Theme cycling now supports "dracula" but the settings UI toggle description still says "Toggle dark/light palette" which is outdated. This should be updated to reflect that there are now three theme options that cycle through dark → light → dracula.
| onActivate: () => | ||
| setSettings((prev) => ({ | ||
| ...prev, | ||
| theme: prev.theme === "dark" ? "light" : "dark", | ||
| })), |
There was a problem hiding this comment.
The theme toggle onActivate function still toggles between only "dark" and "light", but the type system now supports "dracula" as a third option. This creates inconsistent behavior where the user can see "Dracula" as the current value (line 143) but can't cycle to it via the settings toggle. The toggle should implement the same cycling logic as handleThemeCommand (dark → light → dracula → dark).
src/hooks/useAppState.ts
Outdated
| // Save settings when changed | ||
| useEffect(() => { | ||
| saveSettings(settings); | ||
| }, [settings]); | ||
|
|
||
| // Save sessions when changed | ||
| useEffect(() => { | ||
| saveSessions(sessions); | ||
| }, [sessions]); | ||
|
|
||
| // Save custom commands when changed | ||
| useEffect(() => { | ||
| saveCustomCommands(customCommands); | ||
| }, [customCommands]); |
There was a problem hiding this comment.
The saveSettings, saveSessions, and saveCustomCommands functions are called directly in the useEffect hooks. Based on the PR description mentioning "debounced" saving was removed, this may cause performance issues as these functions will be called on every state change. If these functions perform file I/O operations, this could lead to excessive writes. Consider using debounced versions as mentioned in the previous implementation.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Co-authored-by: Zochory <60674042+Zochory@users.noreply.github.com>
Fix hook dependency and performance issues from code review
Description
Type of Change
Related Issues
Fixes #
Related to #
Changes Made
Screenshots (if applicable)
Testing
Test Configuration
Test Cases
bun run typecheck)Checklist
Breaking Changes
Additional Notes
Reviewer Checklist