perf: ⚡ Optimize Chat Workspace Performance#80
Merged
Conversation
… static lists - Reuses already remembered gatewayConfig in sendMessage to avoid redundant allocations. - Extracts the longest static endpoint detail list to a private constant. - Updates Bolt performance journal. - Maintains 50-line diff limit for maintainability. Co-authored-by: yacosta738 <33158051+yacosta738@users.noreply.github.com>
Deploying corvus-plugins with
|
| Latest commit: |
99816d1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bfd2b2b4.corvus-plugins.pages.dev |
| Branch Preview URL: | https://perf-chat-ui-optimization-17.corvus-plugins.pages.dev |
Contributor
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCentralizes Chat workspace UI around new ChatUiState and ChatWorkspaceActions, extracts chat UI primitives into ChatComponents.kt and ConfigPanel.kt, adds material icons dependency, and removes two Scribe documentation/journal files. (26 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant UI as ChatWorkspace UI
participant Actions as ChatWorkspaceActions
participant Config as ConfigPanel
participant Gateway as AgentGatewayConfig
participant Backend as Chat Backend
User->>UI: type query / press send
UI->>Actions: onQueryChange / onSend
Actions->>UI: invoke send handler
UI->>Gateway: read currentGatewayConfig() at invocation
UI->>Backend: send message + gateway config
Backend-->>UI: response
User->>Config: edit baseUrl / token
Config->>Actions: onBaseUrlChange / onBearerTokenChange
Actions->>Gateway: update gateway config state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
corvus-plugins-edge | 99816d1 | Feb 25 2026, 01:08 PM |
Contributor
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-02-25 to 2026-02-25 |
Deploying corvus with
|
| Latest commit: |
99816d1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d98f1888.corvus-42x.pages.dev |
| Branch Preview URL: | https://perf-chat-ui-optimization-17.corvus-42x.pages.dev |
yacosta738
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt (2)
168-181:⚠️ Potential issue | 🔴 CriticalDetekt
LongParameterListandLongMethodfailures are blocking CI — must fix before merge.Three functions exceed the configured threshold of 6 parameters, and
ConfigPaneladditionally exceeds the 60-line method limit:
Function Parameters Lines ChatWorkspaceScreen13 — ChatPanel7 — ConfigPanel6 (at threshold) 89 The standard Compose idiom for reducing parameter count is to introduce a stable wrapper/callback class:
🐛 Suggested structural fix
+@Stable +data class ChatWorkspaceCallbacks( + val onQueryChange: (String) -> Unit, + val onSend: () -> Unit, + val onToggleConfig: () -> Unit, + val onBaseUrlChange: (String) -> Unit, + val onPairingCodeChange: (String) -> Unit, + val onBearerTokenChange: (String) -> Unit, + val onWebhookSecretChange: (String) -> Unit, +) `@Composable` private fun ChatWorkspaceScreen( state: ChatWorkspaceState, messages: List<ChatMessage>, query: String, showConfig: Boolean, gatewayConfig: AgentGatewayConfig, - onQueryChange: (String) -> Unit, - onSend: () -> Unit, - onToggleConfig: () -> Unit, - onBaseUrlChange: (String) -> Unit, - onPairingCodeChange: (String) -> Unit, - onBearerTokenChange: (String) -> Unit, - onWebhookSecretChange: (String) -> Unit, + callbacks: ChatWorkspaceCallbacks, modifier: Modifier = Modifier, )Apply the same pattern to
ChatPanelandConfigPanel. ForConfigPanel's line count, split theLazyColumncontent into a separate private composable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 168 - 181, ChatWorkspaceScreen, ChatPanel, and ConfigPanel violate Detekt size rules; create a stable parameter holder (data class or `@Immutable` wrapper) for grouped callbacks and related fields (e.g., ChatWorkspaceParams or ChatCallbacks containing onQueryChange, onSend, onToggleConfig, and gateway config setters) and replace the long parameter lists in ChatWorkspaceScreen and ChatPanel with this single wrapper to reduce parameter count; for ConfigPanel, extract the LazyColumn content into a new private composable (e.g., ConfigPanelItems or ConfigSettingsList) so ConfigPanel itself stays under the 60-line limit, update all call sites to pass the new wrapper and new composable, and ensure the wrapper is stable/immutable to follow Compose idioms.
350-369:⚠️ Potential issue | 🟠 MajorSensitive credentials (
bearerToken,webhookSecret) are displayed in plaintext.Both fields lack
visualTransformation = PasswordVisualTransformation(), meaning the token and secret are fully visible on screen — a problem on shared displays and in screen recordings.🔒 Proposed fix
+import androidx.compose.ui.text.input.PasswordVisualTransformation OutlinedTextField( value = gatewayConfig.bearerToken, onValueChange = onBearerTokenChange, label = { Text("Bearer token") }, placeholder = { Text("zc_...") }, singleLine = true, + visualTransformation = PasswordVisualTransformation(), modifier = Modifier.fillMaxWidth(), ) OutlinedTextField( value = gatewayConfig.webhookSecret, onValueChange = onWebhookSecretChange, label = { Text("Webhook secret (opcional)") }, placeholder = { Text("X-Webhook-Secret") }, singleLine = true, + visualTransformation = PasswordVisualTransformation(), modifier = Modifier.fillMaxWidth(), )Consider also adding a show/hide toggle button as a
trailingIconfor usability.As per coding guidelines, configuration handling should be checked for security vulnerabilities.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 350 - 369, The OutlinedTextField usages in ChatWorkspace (the fields bound to gatewayConfig.bearerToken and gatewayConfig.webhookSecret) are exposing sensitive tokens in plaintext; update both fields to use visualTransformation = PasswordVisualTransformation() and add a show/hide toggle implemented as a trailingIcon that flips a local boolean (e.g., remember { mutableStateOf(false) }) to switch between PasswordVisualTransformation() and VisualTransformation.None for usability; ensure the toggle uses an IconButton in the trailingIcon and import PasswordVisualTransformation from androidx.compose.ui.text.input.
🧹 Nitpick comments (1)
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt (1)
48-57: Extraction is correct, but/healthand/pairlists are still inline.The
WebhookDetailsextraction achieves the stated goal, but thelistOf(...)blocks for the/health(lines 377–381) and/pair(lines 388–395) endpoint cards remain inline and will still be reallocated on every recomposition. For completeness, extract those too:♻️ Suggested follow-through
private val WebhookDetails = listOf( "Body requerido: {\"message\": \"...\"}", ... ) +private val HealthDetails = listOf( + "Auth: none", + "Respuesta: {\"status\": \"ok\", \"paired\": bool, \"runtime\": {...}}", +) + +private val PairDetails = listOf( + "Header requerido: X-Pairing-Code", + "Sin body JSON", + "Respuesta OK: {\"paired\": true, \"token\": \"zc_...\"}", + "Errores: 403 codigo invalido, 429 rate limit/lockout", +)Then replace the inline
listOf(...)calls at lines 377 and 388 withHealthDetailsandPairDetailsrespectively.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 48 - 57, The /health and /pair card detail lists are still created inline causing reallocations; extract them like WebhookDetails by adding two top-level constants (e.g., HealthDetails and PairDetails) containing the same listOf(...) contents used in the /health and /pair cards, then replace the inline listOf(...) calls in ChatWorkspace (the endpoint card code that currently uses those inline lists) with references to HealthDetails and PairDetails to avoid per-recomposition allocations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/journal/bolt-journal.md:
- Around line 46-47: The benchmark entry currently compares "~10.7s (hot)" vs
"~1.4s (incremental)" which is misleading; update the sentence containing
"~10.7s (hot)" and the Benchmark block to either (A) clarify that "~10.7s" is a
cold/first build time (replace the "(hot)" label) and explicitly show hot and
cold numbers (e.g., include the prior "~2s" hot baseline), or (B) provide
like-for-like comparisons by reporting hot→hot and incremental→incremental
separately (e.g., "hot baseline: ~2s → hot post-optimization: Xs; incremental
baseline: ~10.7s → incremental post-optimization: ~1.4s"), making sure to edit
the exact text fragment "~10.7s (hot)" and the surrounding Benchmark paragraph
in bolt-journal.md accordingly.
---
Outside diff comments:
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 168-181: ChatWorkspaceScreen, ChatPanel, and ConfigPanel violate
Detekt size rules; create a stable parameter holder (data class or `@Immutable`
wrapper) for grouped callbacks and related fields (e.g., ChatWorkspaceParams or
ChatCallbacks containing onQueryChange, onSend, onToggleConfig, and gateway
config setters) and replace the long parameter lists in ChatWorkspaceScreen and
ChatPanel with this single wrapper to reduce parameter count; for ConfigPanel,
extract the LazyColumn content into a new private composable (e.g.,
ConfigPanelItems or ConfigSettingsList) so ConfigPanel itself stays under the
60-line limit, update all call sites to pass the new wrapper and new composable,
and ensure the wrapper is stable/immutable to follow Compose idioms.
- Around line 350-369: The OutlinedTextField usages in ChatWorkspace (the fields
bound to gatewayConfig.bearerToken and gatewayConfig.webhookSecret) are exposing
sensitive tokens in plaintext; update both fields to use visualTransformation =
PasswordVisualTransformation() and add a show/hide toggle implemented as a
trailingIcon that flips a local boolean (e.g., remember { mutableStateOf(false)
}) to switch between PasswordVisualTransformation() and
VisualTransformation.None for usability; ensure the toggle uses an IconButton in
the trailingIcon and import PasswordVisualTransformation from
androidx.compose.ui.text.input.
---
Nitpick comments:
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 48-57: The /health and /pair card detail lists are still created
inline causing reallocations; extract them like WebhookDetails by adding two
top-level constants (e.g., HealthDetails and PairDetails) containing the same
listOf(...) contents used in the /health and /pair cards, then replace the
inline listOf(...) calls in ChatWorkspace (the endpoint card code that currently
uses those inline lists) with references to HealthDetails and PairDetails to
avoid per-recomposition allocations.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.agents/journal/bolt-journal.mdclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt
…intainability - Extracts static endpoint detail lists to constants to avoid redundant allocations. - Reuses remembered gatewayConfig in sendMessage function. - Groups chat callbacks into a stable @immutable ChatWorkspaceActions wrapper. - Refactors ConfigPanel into smaller composables to meet size limits. - Implements PasswordVisualTransformation with visibility toggle for sensitive tokens. - Adds material-icons-extended dependency for UI toggles. - Updates performance journal with like-for-like benchmark comparisons. Co-authored-by: yacosta738 <33158051+yacosta738@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt (4)
152-187:⚠️ Potential issue | 🔴 Critical
sendMessagecaptures a stalegatewayConfig— config changes are silently ignored.
val actions = remember { ChatWorkspaceActions(onSend = ::sendMessage, ...) }evaluates its block only once (initial composition). ThesendMessagelocal function capturesgatewayConfig— avalbound to the return value ofremember(baseUrl, ...). When the user changes any config field,gatewayConfigis re-computed to a newAgentGatewayConfiginstance in the composable scope, but thesendMessagecaptured insideremember {}still holds a reference to the object from the first composition. Every subsequent send will use stale config.The fix is to read config inside
sendMessagedirectly from the underlyingvarstate delegates (which ARE captured by reference and always reflect current values):🐛 Proposed fix
- val gatewayConfig = - remember(baseUrl, pairingCode, bearerToken, webhookSecret) { - AgentGatewayConfig( - baseUrl = baseUrl, - pairingCode = pairingCode, - bearerToken = bearerToken, - webhookSecret = webhookSecret, - ) - } - fun sendMessage() { val prompt = query.trim() if (prompt.isBlank()) { return } + val currentConfig = AgentGatewayConfig( + baseUrl = baseUrl, + pairingCode = pairingCode, + bearerToken = bearerToken, + webhookSecret = webhookSecret, + ) messages.add(ChatMessage(id = nextId, role = ChatRole.User, content = prompt)) nextId += 1 messages.add( ChatMessage( id = nextId, role = ChatRole.Assistant, content = buildLocalAssistantReply( prompt = prompt, modelName = state.modelName, - gateway = gatewayConfig, + gateway = currentConfig, ), ) ) nextId += 1 query = "" }Then pass
gatewayConfig(re-computed viaremember) only to the UI display path (i.e.,ChatWorkspaceScreen), keeping the original remembered value for renderingConfigSettingsList. Alternatively, remove theremember {}wrapper aroundactionsentirely — since all lambda properties capturevardelegates (exceptsendMessagepost-fix), re-creatingChatWorkspaceActionson each recomposition is cheap and correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 152 - 187, sendMessage captured a stale gatewayConfig because actions is created once via remember { ChatWorkspaceActions(...) } so sendMessage closed over the initial gatewayConfig; update sendMessage to read current config from the state-backed vars (e.g., baseUrl, pairingCode, bearerToken, webhookSecret or the var-backed gatewayConfig source) at invocation instead of using the remembered gatewayConfig, and then either (a) pass the recomputed remember(...) gatewayConfig only to UI rendering (ChatWorkspaceScreen/ConfigSettingsList) or (b) remove the remember wrapper so ChatWorkspaceActions (and its onSend = ::sendMessage) are recreated each recomposition; locate functions/vals sendMessage, gatewayConfig, actions, remember and ChatWorkspaceActions to implement the change.
272-279:⚠️ Potential issue | 🟠 MajorFix Detekt
LongParameterList— pipeline failure.
ChatPanelhits the 6-parameter threshold. Replace the two separateChatWorkspaceStatefields with the state object itself:♻️ Proposed fix
`@Composable` private fun ChatPanel( - modelName: String, - inputPlaceholder: String, + state: ChatWorkspaceState, messages: List<ChatMessage>, query: String, actions: ChatWorkspaceActions, modifier: Modifier = Modifier, ) {Update call sites to pass
stateand usestate.modelName/state.inputPlaceholderwithin the composable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 272 - 279, ChatPanel has too many parameters; replace the separate state-related params with the state object. Change the ChatPanel signature to take a single state: ChatWorkspaceState (keep actions: ChatWorkspaceActions and modifier if needed), then inside ChatPanel use state.modelName, state.inputPlaceholder, state.messages, state.query instead of the individual parameters; update all call sites to pass state rather than modelName/inputPlaceholder/messages/query. Ensure imports/reference to ChatWorkspaceState and ChatWorkspaceActions are correct and adjust any usages inside ChatPanel accordingly.
338-428:⚠️ Potential issue | 🟠 MajorFix Detekt
LongMethod(82 lines, max 60) — pipeline failure.
ConfigSettingsListhas two distinct responsibilities: rendering config input fields and rendering the endpoint reference cards. Extract them into separate composables. Additionally, lines 378–420 duplicate the password-field pattern (show/hide icon); a sharedPasswordTextFieldcomposable would eliminate the repetition.♻️ Suggested approach
+@Composable +private fun PasswordTextField( + value: String, + onValueChange: (String) -> Unit, + label: String, + placeholder: String, +) { + var showSecret by remember { mutableStateOf(false) } + OutlinedTextField( + value = value, + onValueChange = onValueChange, + label = { Text(label) }, + placeholder = { Text(placeholder) }, + singleLine = true, + modifier = Modifier.fillMaxWidth(), + visualTransformation = if (showSecret) VisualTransformation.None else PasswordVisualTransformation(), + trailingIcon = { + IconButton(onClick = { showSecret = !showSecret }) { + Icon( + imageVector = if (showSecret) Icons.Default.VisibilityOff else Icons.Default.Visibility, + contentDescription = if (showSecret) "Hide" else "Show", + ) + } + }, + ) +}Then replace the duplicated
bearerTokenandwebhookSecretOutlinedTextFieldblocks withPasswordTextField(...)calls, and extract the threeEndpointCarditems into anEndpointsSectioncomposable to keep each function under 60 lines.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 338 - 428, ConfigSettingsList is too long and duplicates password-field logic; extract the repeated OutlinedTextField + visibility toggle into a reusable PasswordTextField composable (accepting value, onValueChange, label, placeholder) and move the three EndpointCard items into an EndpointsSection composable (accepting the computed healthUrl/pairUrl/webhookUrl and the HealthDetails/PairDetails/WebhookDetails references). Replace the bearerToken and webhookSecret OutlinedTextField blocks in ConfigSettingsList with PasswordTextField(gatewayConfig.bearerToken, actions.onBearerTokenChange, ...) and PasswordTextField(gatewayConfig.webhookSecret, actions.onWebhookSecretChange, ...), and replace the three EndpointCard items with a single EndpointsSection(...) call; keep the show/hide state inside PasswordTextField using remember so ConfigSettingsList stays under 60 lines.
200-209:⚠️ Potential issue | 🟠 MajorFix Detekt
LongParameterList— pipeline failure.
ChatWorkspaceScreenhas 7 parameters against a threshold of 6. Consolidatemessages,query, andshowConfigalongside the existingstateinto a broaderChatUiState(or similar) wrapper to bring parameter count within the limit and make the API more cohesive.♻️ Suggested approach
+@Immutable +private data class ChatUiState( + val workspace: ChatWorkspaceState, + val messages: List<ChatMessage>, + val query: String, + val showConfig: Boolean, + val gatewayConfig: AgentGatewayConfig, +) `@Composable` private fun ChatWorkspaceScreen( - state: ChatWorkspaceState, - messages: List<ChatMessage>, - query: String, - showConfig: Boolean, - gatewayConfig: AgentGatewayConfig, + uiState: ChatUiState, actions: ChatWorkspaceActions, modifier: Modifier = Modifier, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 200 - 209, ChatWorkspaceScreen exceeds Detekt's LongParameterList; create a new wrapper data class (e.g., ChatUiState) that contains the existing ChatWorkspaceState plus messages: List<ChatMessage>, query: String, and showConfig: Boolean, then change the composable signature from ChatWorkspaceScreen(state: ChatWorkspaceState, messages: List<ChatMessage>, query: String, showConfig: Boolean, ...) to ChatWorkspaceScreen(uiState: ChatUiState, gatewayConfig: AgentGatewayConfig, actions: ChatWorkspaceActions, modifier: Modifier = Modifier), update all call sites to construct and pass ChatUiState, and adjust usages inside ChatWorkspaceScreen to read uiState.workspaceState / uiState.messages / uiState.query / uiState.showConfig.
🧹 Nitpick comments (2)
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt (2)
328-336:BorderStrokeinConfigPanelis re-allocated on every recomposition.
ChatPanelwraps itsBorderStrokeinremember(colors.outline) { ... }to avoid redundant allocations.ConfigPanelcreates theBorderStrokeinline, inconsistent with the PR's goal of reducing object churn.♻️ Proposed fix
`@Composable` private fun ConfigPanel( gatewayConfig: AgentGatewayConfig, actions: ChatWorkspaceActions, modifier: Modifier = Modifier, ) { + val colors = MaterialTheme.colorScheme + val borderStroke = remember(colors.outline) { + BorderStroke(1.dp, colors.outline.copy(alpha = 0.3f)) + } Surface( modifier = modifier.fillMaxWidth(), shape = ConfigPanelShape, - border = BorderStroke(1.dp, MaterialTheme.colorScheme.outline.copy(alpha = 0.3f)), - color = MaterialTheme.colorScheme.surface, + border = borderStroke, + color = colors.surface, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 328 - 336, ConfigPanel currently creates a new BorderStroke on every recomposition; change it to cache the BorderStroke using remember with the outline color (similar to ChatPanel) so the BorderStroke is not re-allocated each recomposition—update the BorderStroke creation inside ConfigPanel (the Surface call around ConfigSettingsList and ConfigPanelShape) to use remember(colors.outline) { BorderStroke(1.dp, colors.outline.copy(alpha = 0.3f)) } and pass that remembered instance to the Surface border parameter.
57-75: ConsiderSCREAMING_SNAKE_CASEfor static constant values.
HealthDetails,PairDetails, andWebhookDetailsare static constants. Kotlin conventions recommendSCREAMING_SNAKE_CASEfor top-levelvals that hold constant data (e.g.,HEALTH_DETAILS), to distinguish them from types/classes at a glance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 57 - 75, Rename the top-level vals HealthDetails, PairDetails, and WebhookDetails to SCREAMING_SNAKE_CASE (HEALTH_DETAILS, PAIR_DETAILS, WEBHOOK_DETAILS) to follow Kotlin constant naming conventions; update all usages in ChatWorkspace (and any imports/usages in the same module) to the new names and ensure visibility remains the same (keep them as top-level vals) so behavior doesn't change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/composeApp/build.gradle.kts`:
- Line 53: The project currently depends on the deprecated and heavy
compose.materialIconsExtended; replace that dependency with the smaller core
icons artifact because the code only uses Icons.Default.Visibility and
Icons.Default.VisibilityOff. Update the Gradle dependency entry that references
compose.materialIconsExtended to instead add the explicit material-icons-core
artifact (or switch to Material Symbols vector resources if you prefer),
ensuring imports for Icons.Default.Visibility and Icons.Default.VisibilityOff
continue to resolve against the core icon library.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 92-101: The ChatWorkspaceActions data class is incorrectly
annotated with `@Immutable`; change the annotation to `@Stable` on
ChatWorkspaceActions to reflect that its lambda properties capture mutable state
and do not guarantee deep immutability — locate the ChatWorkspaceActions
declaration and replace `@Immutable` with `@Stable` so Compose treats the type as
stable without overpromising immutability.
---
Outside diff comments:
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 152-187: sendMessage captured a stale gatewayConfig because
actions is created once via remember { ChatWorkspaceActions(...) } so
sendMessage closed over the initial gatewayConfig; update sendMessage to read
current config from the state-backed vars (e.g., baseUrl, pairingCode,
bearerToken, webhookSecret or the var-backed gatewayConfig source) at invocation
instead of using the remembered gatewayConfig, and then either (a) pass the
recomputed remember(...) gatewayConfig only to UI rendering
(ChatWorkspaceScreen/ConfigSettingsList) or (b) remove the remember wrapper so
ChatWorkspaceActions (and its onSend = ::sendMessage) are recreated each
recomposition; locate functions/vals sendMessage, gatewayConfig, actions,
remember and ChatWorkspaceActions to implement the change.
- Around line 272-279: ChatPanel has too many parameters; replace the separate
state-related params with the state object. Change the ChatPanel signature to
take a single state: ChatWorkspaceState (keep actions: ChatWorkspaceActions and
modifier if needed), then inside ChatPanel use state.modelName,
state.inputPlaceholder, state.messages, state.query instead of the individual
parameters; update all call sites to pass state rather than
modelName/inputPlaceholder/messages/query. Ensure imports/reference to
ChatWorkspaceState and ChatWorkspaceActions are correct and adjust any usages
inside ChatPanel accordingly.
- Around line 338-428: ConfigSettingsList is too long and duplicates
password-field logic; extract the repeated OutlinedTextField + visibility toggle
into a reusable PasswordTextField composable (accepting value, onValueChange,
label, placeholder) and move the three EndpointCard items into an
EndpointsSection composable (accepting the computed healthUrl/pairUrl/webhookUrl
and the HealthDetails/PairDetails/WebhookDetails references). Replace the
bearerToken and webhookSecret OutlinedTextField blocks in ConfigSettingsList
with PasswordTextField(gatewayConfig.bearerToken, actions.onBearerTokenChange,
...) and PasswordTextField(gatewayConfig.webhookSecret,
actions.onWebhookSecretChange, ...), and replace the three EndpointCard items
with a single EndpointsSection(...) call; keep the show/hide state inside
PasswordTextField using remember so ConfigSettingsList stays under 60 lines.
- Around line 200-209: ChatWorkspaceScreen exceeds Detekt's LongParameterList;
create a new wrapper data class (e.g., ChatUiState) that contains the existing
ChatWorkspaceState plus messages: List<ChatMessage>, query: String, and
showConfig: Boolean, then change the composable signature from
ChatWorkspaceScreen(state: ChatWorkspaceState, messages: List<ChatMessage>,
query: String, showConfig: Boolean, ...) to ChatWorkspaceScreen(uiState:
ChatUiState, gatewayConfig: AgentGatewayConfig, actions: ChatWorkspaceActions,
modifier: Modifier = Modifier), update all call sites to construct and pass
ChatUiState, and adjust usages inside ChatWorkspaceScreen to read
uiState.workspaceState / uiState.messages / uiState.query / uiState.showConfig.
---
Nitpick comments:
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 328-336: ConfigPanel currently creates a new BorderStroke on every
recomposition; change it to cache the BorderStroke using remember with the
outline color (similar to ChatPanel) so the BorderStroke is not re-allocated
each recomposition—update the BorderStroke creation inside ConfigPanel (the
Surface call around ConfigSettingsList and ConfigPanelShape) to use
remember(colors.outline) { BorderStroke(1.dp, colors.outline.copy(alpha = 0.3f))
} and pass that remembered instance to the Surface border parameter.
- Around line 57-75: Rename the top-level vals HealthDetails, PairDetails, and
WebhookDetails to SCREAMING_SNAKE_CASE (HEALTH_DETAILS, PAIR_DETAILS,
WEBHOOK_DETAILS) to follow Kotlin constant naming conventions; update all usages
in ChatWorkspace (and any imports/usages in the same module) to the new names
and ensure visibility remains the same (keep them as top-level vals) so behavior
doesn't change.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.agents/command/scribe.agent.md.agents/journal/bolt-journal.md.agents/journal/scribe-journal.mdclients/composeApp/build.gradle.ktsclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt
💤 Files with no reviewable changes (2)
- .agents/journal/scribe-journal.md
- .agents/command/scribe.agent.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .agents/journal/bolt-journal.md
…ity and code quality - Refactors ChatWorkspace to use ChatUiState and ChatWorkspaceActions wrappers, reducing parameter count and improving skippability. - Fixes stale state capture in sendMessage by reading state variables at invocation time. - Implements PasswordTextField with visibility toggles for sensitive fields (bearerToken, webhookSecret). - Extracts EndpointsSection and static details lists (SCREAMING_SNAKE_CASE) to reduce component size and allocations. - Caches BorderStroke in ConfigPanel using remember. - Updates Bolt performance journal with like-for-like comparisons. - Note: materialIconsExtended is kept as core icons artifact was unable to resolve Visibility icons in this environment. Co-authored-by: yacosta738 <33158051+yacosta738@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt (2)
454-486:⚠️ Potential issue | 🟡 Minor
BorderStrokeinEndpointCardis allocated on every recomposition — wrap inremember.
BorderStroke(1.dp, MaterialTheme.colorScheme.outline.copy(alpha = 0.25f))at line 460 and the same pattern inChatBubbleat line 501 are both allocated on every recomposition. Given that this PR is explicitly focused on reducing allocations during recomposition, these are inconsistent with the stated goal. Cache them withremember(colors.outline)the same wayChatPanelandConfigPanelalready do.♻️ Proposed fix for `EndpointCard`
`@Composable` private fun EndpointCard(title: String, subtitle: String, details: List<String>) { + val colors = MaterialTheme.colorScheme + val borderStroke = remember(colors.outline) { BorderStroke(1.dp, colors.outline.copy(alpha = 0.25f)) } Surface( modifier = Modifier.fillMaxWidth(), shape = EndpointCardShape, - color = MaterialTheme.colorScheme.surfaceVariant.copy(alpha = 0.2f), - border = BorderStroke(1.dp, MaterialTheme.colorScheme.outline.copy(alpha = 0.25f)), + color = colors.surfaceVariant.copy(alpha = 0.2f), + border = borderStroke, ) {♻️ Proposed fix for `ChatBubble`
`@Composable` private fun ChatBubble(message: ChatMessage, modelName: String) { val isUser = message.role == ChatRole.User val colors = MaterialTheme.colorScheme + val borderStroke = remember(colors.outline) { BorderStroke(1.dp, colors.outline.copy(alpha = 0.2f)) } Row(...) { Surface( ... - border = BorderStroke(1.dp, colors.outline.copy(alpha = 0.2f)), + border = borderStroke, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 454 - 486, The BorderStroke instance used in EndpointCard (BorderStroke(1.dp, MaterialTheme.colorScheme.outline.copy(alpha = 0.25f))) and the similar allocation in ChatBubble should be wrapped in remember to avoid re-allocating on every recomposition; update EndpointCard and ChatBubble to create a remembered stroke (e.g. val outlineStroke = remember(MaterialTheme.colorScheme.outline) { BorderStroke(1.dp, MaterialTheme.colorScheme.outline.copy(alpha = 0.25f)) }) and then pass outlineStroke into the Surface border parameter, matching the caching pattern used in ChatPanel/ConfigPanel.
1-1:⚠️ Potential issue | 🔴 CriticalDetekt pipeline failure:
TooManyFunctions— split the file.The CI pipeline reports 12 functions in this file against a threshold of 11. The file now contains
ChatWorkspace,ChatWorkspaceScreen,ChatHeader,ChatPanel,ConfigPanel,ConfigSettingsList,PasswordTextField,EndpointsSection,EndpointCard,ChatBubble,endpointUrl, andbuildLocalAssistantReply. The most natural split is to move the reusable/standalone composables (PasswordTextField,EndpointsSection,EndpointCard,ChatBubble) or the config-related group (ConfigPanel,ConfigSettingsList,PasswordTextField,EndpointsSection) into a sibling file (e.g.,ChatComponents.ktorConfigPanel.kt).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` at line 1, The file exceeds Detekt's TooManyFunctions limit—split reusable/standalone composables into a sibling file: move PasswordTextField, EndpointsSection, EndpointCard, and ChatBubble (or alternatively the whole config group: ConfigPanel and ConfigSettingsList plus PasswordTextField and EndpointsSection) out of ChatWorkspace.kt into a new file (e.g., ChatComponents.kt or ConfigPanel.kt); update the package, imports, and any local references so ChatWorkspace, ChatWorkspaceScreen, ChatHeader, ChatPanel, ConfigPanel, ConfigSettingsList, endpointUrl, and buildLocalAssistantReply continue to compile with the moved composables now declared in the new file. Ensure no behavior changes and run Detekt to confirm the function count is under the threshold.
🧹 Nitpick comments (1)
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt (1)
208-209:uiStateis reallocated on every recomposition ofChatWorkspace.
ChatUiState(...)is constructed unconditionally at lines 208-209 with noremember. Sincemessagesis aSnapshotStateList(reference-stable) andquery/showConfigare primitive state values, the allocation is cheap but still creates a new wrapper object every frame. Wrapping it inremember(query, showConfig)(passingmessagesby reference since it is already a stableSnapshotStateList) would skip the allocation on frames where neither changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 208 - 209, The ChatWorkspace builds a new ChatUiState each recomposition (uiState = ChatUiState(...)), causing unnecessary allocations; wrap the construction in a compose remember block so the ChatUiState is only recreated when its inputs change — e.g., use remember(query, showConfig, messages) or remember(query, showConfig) (passing messages by reference since it's a SnapshotStateList) to memoize the ChatUiState creation; update the uiState assignment accordingly in the ChatWorkspace function so ChatUiState is preserved across recompositions unless query, showConfig or messages change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 85-91: The ChatUiState data class is incorrectly annotated with
`@Immutable` even though its messages property is a SnapshotStateList created via
mutableStateListOf in ChatWorkspace, so replace `@Immutable` with `@Stable` on
ChatUiState to reflect its stable-but-mutable snapshot-backed list; locate the
ChatUiState declaration and change the annotation to `@Stable`, ensuring you do
not mark properties as deeply immutable (leave messages: List<ChatMessage>
as-is) so Compose will correctly observe mutations to the SnapshotStateList.
---
Outside diff comments:
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 454-486: The BorderStroke instance used in EndpointCard
(BorderStroke(1.dp, MaterialTheme.colorScheme.outline.copy(alpha = 0.25f))) and
the similar allocation in ChatBubble should be wrapped in remember to avoid
re-allocating on every recomposition; update EndpointCard and ChatBubble to
create a remembered stroke (e.g. val outlineStroke =
remember(MaterialTheme.colorScheme.outline) { BorderStroke(1.dp,
MaterialTheme.colorScheme.outline.copy(alpha = 0.25f)) }) and then pass
outlineStroke into the Surface border parameter, matching the caching pattern
used in ChatPanel/ConfigPanel.
- Line 1: The file exceeds Detekt's TooManyFunctions limit—split
reusable/standalone composables into a sibling file: move PasswordTextField,
EndpointsSection, EndpointCard, and ChatBubble (or alternatively the whole
config group: ConfigPanel and ConfigSettingsList plus PasswordTextField and
EndpointsSection) out of ChatWorkspace.kt into a new file (e.g.,
ChatComponents.kt or ConfigPanel.kt); update the package, imports, and any local
references so ChatWorkspace, ChatWorkspaceScreen, ChatHeader, ChatPanel,
ConfigPanel, ConfigSettingsList, endpointUrl, and buildLocalAssistantReply
continue to compile with the moved composables now declared in the new file.
Ensure no behavior changes and run Detekt to confirm the function count is under
the threshold.
---
Nitpick comments:
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 208-209: The ChatWorkspace builds a new ChatUiState each
recomposition (uiState = ChatUiState(...)), causing unnecessary allocations;
wrap the construction in a compose remember block so the ChatUiState is only
recreated when its inputs change — e.g., use remember(query, showConfig,
messages) or remember(query, showConfig) (passing messages by reference since
it's a SnapshotStateList) to memoize the ChatUiState creation; update the
uiState assignment accordingly in the ChatWorkspace function so ChatUiState is
preserved across recompositions unless query, showConfig or messages change.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt
…structure - Fixes stale state capture in sendMessage by reading state variables at invocation time. - Memoizes ChatUiState construction and all BorderStroke allocations using remember. - Splits ChatWorkspace.kt into ConfigPanel.kt and ChatComponents.kt to comply with Detekt rules. - Uses @stable for state-backed wrappers to optimize recomposition skipping. - Standardizes constant naming to screaming snake case. - Updates performance journal with architectural optimizations. Co-authored-by: yacosta738 <33158051+yacosta738@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt (2)
63-72: Past review feedback addressed —@Stableis correct here.
ChatWorkspaceActionsnow correctly uses@Stablesince its lambda properties capture mutable state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 63 - 72, ChatWorkspaceActions is correctly annotated with `@Stable` because its lambda properties capture mutable state; leave the `@Stable` annotation on the ChatWorkspaceActions data class as-is and no further changes are needed to the onQueryChange/onSend/onToggleConfig/onBaseUrlChange/onPairingCodeChange/onBearerTokenChange/onWebhookSecretChange properties.
47-53: Past review feedback addressed —@Stableis correct here.
ChatUiStatenow uses@Stableinstead of@Immutable, which is appropriate given theSnapshotStateList-backedmessages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 47 - 53, Keep the `@Stable` annotation on ChatUiState (do not revert to `@Immutable`); ensure the messages property is backed by a SnapshotStateList<ChatMessage> (or another Compose snapshot-aware list) and that ChatWorkspaceState / ChatMessage types remain unchanged, and remove any duplicate review comment markers left in the file or PR metadata (the duplicate [duplicate_comment] tag) so the approval stands cleanly.
🧹 Nitpick comments (5)
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt (2)
163-171:messagesas arememberkey is a no-op here.
messagesis aSnapshotStateListwhose identity never changes when items are added or removed — only its contents do. Including it as a key inremember(state, messages, query, showConfig)gives a false sense of reactivity;uiStatewill not be re-created when messages change.In practice this is fine because downstream composables observe the
SnapshotStateListdirectly (items read trigger snapshot subscriptions). But the key is misleading. Consider removing it for clarity:- val uiState = - remember(state, messages, query, showConfig) { + val uiState = + remember(state, query, showConfig) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 163 - 171, The remember call for uiState incorrectly includes the SnapshotStateList messages as a key (in remember(state, messages, query, showConfig)), which is a no-op because messages' identity doesn't change; remove messages from the remember keys and use remember(state, query, showConfig) so uiState isn't misleadingly tied to messages' identity—if you actually need uiState to update when the list contents change, use a content-derived key like messages.size or messages.toList() instead; update the call that constructs ChatUiState accordingly.
182-182: DetektFunctionNamingwarning is a false positive for@Composablefunctions.The pipeline reports
FunctionNamingonChatWorkspaceScreen. Compose convention requires PascalCase for@Composablefunctions that emit UI. Consider suppressing this rule for Compose files in your Detekt config, or adding a file-level@Suppress("FunctionNaming").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` at line 182, Detekt flags ChatWorkspaceScreen for FunctionNaming even though it’s an `@Composable` UI function; fix by suppressing the rule for Compose files: add a file-level suppression (e.g. `@file`:Suppress("FunctionNaming")) at the top of this Kotlin file or update the Detekt configuration to exempt `@Composable/PascalCase` functions, and ensure the suppression targets the ChatWorkspaceScreen symbol and other composables in this file.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt (2)
143-175:ChatBubbleis well-structured — consider accessibility of the hardcoded320.dpmax width.The bubble caps at
320.dp. On very small screens or large font scale settings this might clip long content. AfillMaxWidth(0.75f)approach is more responsive, though this depends on your target form factors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt` around lines 143 - 175, The fixed max width of 320.dp in ChatBubble (modifier on Surface) can cause clipping on small screens or large font scales; replace the hardcoded Modifier.widthIn(max = 320.dp) with a responsive constraint such as Modifier.fillMaxWidth(0.75f) or a combined approach (e.g., fillMaxWidth(0.75f).widthIn(max = someDp)) so the ChatBubbleShape and content (Text message.content) can wrap appropriately across form factors.
177-183:endpointUrlassumespathstarts with/.The function concatenates
normalizedBaseandpathdirectly. Ifpathever omits the leading/, the resulting URL would be malformed (e.g."http://localhost:3000webhook"). All current call sites use"/health","/pair","/webhook"so this is safe today, but a defensive guard would harden it:♻️ Optional defensive normalization
internal fun endpointUrl(baseUrl: String, path: String): String { val normalizedBase = baseUrl.trim().removeSuffix("/") + val normalizedPath = if (path.startsWith("/")) path else "/$path" if (normalizedBase.isEmpty()) { - return path + return normalizedPath } - return "$normalizedBase$path" + return "$normalizedBase$normalizedPath" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt` around lines 177 - 183, endpointUrl concatenates normalizedBase and path assuming path starts with '/', which can produce malformed URLs if callers omit the leading slash; update endpointUrl to defensively normalize path (e.g., compute a normalizedPath from path by trimming and ensuring it begins with '/' then use normalizedPath in the return) and adjust the empty-base branch to return normalizedPath instead of raw path; reference the function endpointUrl and the local variables normalizedBase and path when making the change.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.kt (1)
38-94: Consider accepting aModifierparameter onConfigSettingsList.Internal composables that emit layout content should conventionally accept a
Modifierparameter for flexibility. CurrentlyConfigSettingsListhardcodesModifier.fillMaxSize().padding(14.dp)on itsLazyColumn, making it harder for callers to override sizing or padding.♻️ Suggested change
`@Composable` -internal fun ConfigSettingsList(gatewayConfig: AgentGatewayConfig, actions: ChatWorkspaceActions) { +internal fun ConfigSettingsList( + gatewayConfig: AgentGatewayConfig, + actions: ChatWorkspaceActions, + modifier: Modifier = Modifier, +) { val (healthUrl, pairUrl, webhookUrl) = ... LazyColumn( - modifier = Modifier.fillMaxSize().padding(14.dp), + modifier = modifier.fillMaxSize().padding(14.dp), verticalArrangement = Arrangement.spacedBy(10.dp), ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.kt` around lines 38 - 94, ConfigSettingsList currently hardcodes layout modifiers on its LazyColumn which prevents callers from customizing layout; change the signature of ConfigSettingsList to accept a Modifier parameter (e.g., modifier: Modifier = Modifier), use that modifier when building the LazyColumn (combine it with .fillMaxSize().padding(14.dp) so callers can override/augment), and update any call sites of ConfigSettingsList to pass a modifier when needed; ensure you reference ConfigSettingsList and the LazyColumn modifier composition so consumers can control sizing/padding.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/journal/bolt-journal.md:
- Around line 39-40: Update the journal entries to remove the apparent
contradiction about gatewayConfig by adding a short “superseded in next
iteration” note to the earlier bullet that says you moved the remember'ed
gatewayConfig above sendMessage and reused it; mention that this approach was
later replaced by reading the current state at invocation to avoid
stale-capture. Do the same clarification for the related entry around lines
57-63; also keep the extracted WebhookDetails note as-is but state it is
unrelated to the gatewayConfig change to avoid confusion with the composable and
sendMessage descriptions.
- Line 69: Update the typo in the journal bullet by replacing the string
"Skipiability" with the correct "Skippability" in the entry that reads
"**Maximized Skipiability**: Optimized Compose compiler's ability to skip
recompositions via stable wrappers and memoization." (search for that exact
bullet text in bolt-journal.md and change only the misspelled word).
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 151-161: The current remember block creates actions once and
captures the initial state in the ::sendMessage reference, causing sendMessage
(used by ChatWorkspaceActions) to hold a stale state.modelName; to fix, make the
remembered actions read the up-to-date state by either adding state as a key to
the remember call (remember(state) { ... }) so actions and the ::sendMessage
binding are recreated when state changes, or keep a single allocation and inside
the composable use rememberUpdatedState(state) (and have sendMessage read the
latestState.value.modelName) so ChatWorkspaceActions/onSend always uses the
current state.modelName.
---
Duplicate comments:
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 63-72: ChatWorkspaceActions is correctly annotated with `@Stable`
because its lambda properties capture mutable state; leave the `@Stable`
annotation on the ChatWorkspaceActions data class as-is and no further changes
are needed to the
onQueryChange/onSend/onToggleConfig/onBaseUrlChange/onPairingCodeChange/onBearerTokenChange/onWebhookSecretChange
properties.
- Around line 47-53: Keep the `@Stable` annotation on ChatUiState (do not revert
to `@Immutable`); ensure the messages property is backed by a
SnapshotStateList<ChatMessage> (or another Compose snapshot-aware list) and that
ChatWorkspaceState / ChatMessage types remain unchanged, and remove any
duplicate review comment markers left in the file or PR metadata (the duplicate
[duplicate_comment] tag) so the approval stands cleanly.
---
Nitpick comments:
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt`:
- Around line 143-175: The fixed max width of 320.dp in ChatBubble (modifier on
Surface) can cause clipping on small screens or large font scales; replace the
hardcoded Modifier.widthIn(max = 320.dp) with a responsive constraint such as
Modifier.fillMaxWidth(0.75f) or a combined approach (e.g.,
fillMaxWidth(0.75f).widthIn(max = someDp)) so the ChatBubbleShape and content
(Text message.content) can wrap appropriately across form factors.
- Around line 177-183: endpointUrl concatenates normalizedBase and path assuming
path starts with '/', which can produce malformed URLs if callers omit the
leading slash; update endpointUrl to defensively normalize path (e.g., compute a
normalizedPath from path by trimming and ensuring it begins with '/' then use
normalizedPath in the return) and adjust the empty-base branch to return
normalizedPath instead of raw path; reference the function endpointUrl and the
local variables normalizedBase and path when making the change.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 163-171: The remember call for uiState incorrectly includes the
SnapshotStateList messages as a key (in remember(state, messages, query,
showConfig)), which is a no-op because messages' identity doesn't change; remove
messages from the remember keys and use remember(state, query, showConfig) so
uiState isn't misleadingly tied to messages' identity—if you actually need
uiState to update when the list contents change, use a content-derived key like
messages.size or messages.toList() instead; update the call that constructs
ChatUiState accordingly.
- Line 182: Detekt flags ChatWorkspaceScreen for FunctionNaming even though it’s
an `@Composable` UI function; fix by suppressing the rule for Compose files: add a
file-level suppression (e.g. `@file`:Suppress("FunctionNaming")) at the top of
this Kotlin file or update the Detekt configuration to exempt
`@Composable/PascalCase` functions, and ensure the suppression targets the
ChatWorkspaceScreen symbol and other composables in this file.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.kt`:
- Around line 38-94: ConfigSettingsList currently hardcodes layout modifiers on
its LazyColumn which prevents callers from customizing layout; change the
signature of ConfigSettingsList to accept a Modifier parameter (e.g., modifier:
Modifier = Modifier), use that modifier when building the LazyColumn (combine it
with .fillMaxSize().padding(14.dp) so callers can override/augment), and update
any call sites of ConfigSettingsList to pass a modifier when needed; ensure you
reference ConfigSettingsList and the LazyColumn modifier composition so
consumers can control sizing/padding.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.agents/journal/bolt-journal.mdclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.kt
…ural refactor - Performance: Groups state into @stable wrappers (ChatUiState, ChatWorkspaceActions) and memoizes their creation using remember with correct keys. - Performance: Caches all BorderStroke allocations across UI components to avoid re-allocation. - Performance: Extracts static string lists to screaming snake case constants. - Bug Fix: Prevents stale state capture in sendMessage by reading state variables at invocation time and using 'state' as a remember key for actions. - Security: Masks sensitive fields (bearerToken, webhookSecret) with show/hide toggle. - Architecture: Splits monolithic ChatWorkspace.kt into ChatComponents.kt and ConfigPanel.kt to comply with Detekt's function count and size limits. - Code Quality: Implements file-level suppression for Compose function naming rules and ensures robust URL path normalization. - UI: Improves responsiveness by replacing fixed widths with percentage-based constraints for chat bubbles. - Documentation: Updates bolt-journal.md with accurate, like-for-like performance metrics. Co-authored-by: yacosta738 <33158051+yacosta738@users.noreply.github.com>
# Conflicts: # .agents/journal/scribe-journal.md
yacosta738
deleted the
perf-chat-ui-optimization-17235947650600110026
branch
Merged
This was referenced Mar 18, 2026
This was referenced Mar 28, 2026
This was referenced Apr 9, 2026
Merged
Merged
Merged
This was referenced Apr 19, 2026
This was referenced Apr 29, 2026
8 tasks
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Optimized the
ChatWorkspaceCompose component by reducing unnecessary object allocations during runtime interactions. Specifically, I moved therememberedgatewayConfigto a scope where it can be reused by thesendMessagefunction, and extracted the staticWebhookDetailslist to a top-level constant. These changes reduce GC pressure and improve UI smoothness when typing in the configuration panel. Verified with:composeApp:checkand incremental build tests.PR created automatically by Jules for task 17235947650600110026 started by @yacosta738
Summary by CodeRabbit
New Features
Bug Fixes / Stability
Chores
Documentation