refactor(agents-ui): split AgentsApp.tsx into apps/agents/ components (B6a)#558
Merged
Conversation
📝 WalkthroughWalkthroughThis PR modularizes the AgentsApp by extracting previously inline component implementations into dedicated, reusable modules. It establishes shared types (Agent, DiskState, ArchivedAgent, Framework) and constants (MEMORY_STEPS_MB, COLORS, STATUS_STYLES), then exports AgentRow, AgentDetailPanel, DeployWizard, and ArchivedAgentsPanel as standalone components, reducing the main file from 2055 to approximately 8 lines of imports while preserving the same public API. ChangesAgent Management UI Modularization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
desktop/src/apps/agents/constants.ts (1)
12-17: ⚡ Quick winMake status-style mapping exhaustive against
Agent.status.Typing this as
Record<string, string>weakens compile-time checks. UseRecord<Agent["status"], string>(oras const+satisfies) so missing/typoed status keys fail at build time.Proposed diff
+import type { Agent } from "./types"; + -export const STATUS_STYLES: Record<string, string> = { +export const STATUS_STYLES: Record<Agent["status"], string> = { running: "bg-emerald-500/20 text-emerald-400", stopped: "bg-zinc-500/20 text-zinc-400", error: "bg-red-500/20 text-red-400", deploying: "bg-amber-500/20 text-amber-400", };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/agents/constants.ts` around lines 12 - 17, STATUS_STYLES is currently typed as Record<string,string>, which hides missing/typoed keys for Agent.status; change its typing to be exhaustive by declaring it as Record<Agent["status"], string> or use an explicit const object with satisfies Record<Agent["status"], string> so the compiler enforces all Agent.status values are present (update the STATUS_STYLES declaration and imports so Agent type is available where STATUS_STYLES is defined).desktop/src/apps/agents/types.ts (1)
1-1: ⚡ Quick winDecouple shared types from UI component-owned model type.
desktop/src/apps/agents/types.tsimportingAgentModelfrom@/components/ModelPickerFlowcouples a shared domain-types module to a UI flow module. Please moveAgentModelto a neutral types module (for exampledesktop/src/types/agent-model.ts) and import it from both places.Also applies to: 60-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/agents/types.ts` at line 1, The shared types module is incorrectly coupled to a UI component by importing AgentModel from ModelPickerFlow; extract the AgentModel type into a neutral types module (e.g., create agent-model.ts exporting AgentModel), update the ModelPickerFlow component to import AgentModel from that new module, and change desktop/src/apps/agents/types.ts to import AgentModel from the new neutral module instead of "`@/components/ModelPickerFlow`"; ensure the new module exports the same type name and update any other imports (lines referenced around 60–61) to use the new module.desktop/src/apps/agents/AgentDetailPanel.tsx (1)
60-62: 💤 Low valueConsider capturing the error for better diagnostics.
The catch block doesn't capture the error parameter, so if the fetch fails (e.g., network error, CORS issue), the user only sees a generic message. While the current message is acceptable, including the error type or message would help with debugging.
♻️ Optional enhancement to include error details
- } catch { + } catch (err) { setLogs(`[${new Date().toLocaleTimeString()}] Unable to reach log endpoint for ${agentName}.\n[${new Date().toLocaleTimeString()}] Agent may not be running or the API is unavailable.`); }Or include the error in the message:
- } catch { - setLogs(`[${new Date().toLocaleTimeString()}] Unable to reach log endpoint for ${agentName}.\n[${new Date().toLocaleTimeString()}] Agent may not be running or the API is unavailable.`); + } catch (err) { + setLogs(`[${new Date().toLocaleTimeString()}] Unable to reach log endpoint for ${agentName}: ${err}\n[${new Date().toLocaleTimeString()}] Agent may not be running or the API is unavailable.`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@desktop/src/apps/agents/AgentDetailPanel.tsx` around lines 60 - 62, The catch block that calls setLogs when fetching logs should capture the thrown error (e.g., catch (err)) and include its details in the log message to aid diagnostics; update the handler around setLogs (the code using setLogs and agentName in AgentDetailPanel) to append or interpolate err.message (and optionally err.name or JSON.stringify(err)) into the timestamped message and also consider console.error(err) for developer visibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/src/apps/agents/AgentDetailPanel.tsx`:
- Around line 78-81: The dismissMigrationBanner function lacks error handling so
a failed POST still triggers onAgentUpdated; wrap the fetch in a try/catch,
await the response and check res.ok, call onAgentUpdated only on success, and
handle failures by logging the error and/or surfacing a user-visible error
state; update the function named dismissMigrationBanner to perform the fetch ->
if (!res.ok) throw or handle error -> on success call onAgentUpdated, and catch
any exceptions to log/report them.
In `@desktop/src/apps/agents/DeployWizard.tsx`:
- Around line 754-756: DeployWizard is sending kv_cache_quant_k,
kv_cache_quant_v, and kv_cache_quant_boundary_layers but DeployAgentRequest and
deploy_agent_endpoint aren’t persisting them; add these three fields to the
DeployAgentRequest Pydantic model in tinyagentos/routes/agents.py (matching
types used in the frontend), then update deploy_agent_endpoint to copy
kv_cache_quant_k, kv_cache_quant_v, and kv_cache_quant_boundary_layers into the
agent config/dict that you pass to normalize_agent (tinyagentos/config.py) so
normalize_agent receives and preserves these values (and ensure any type/field
names match normalize_agent’s expectations or extend normalize_agent to accept
them).
---
Nitpick comments:
In `@desktop/src/apps/agents/AgentDetailPanel.tsx`:
- Around line 60-62: The catch block that calls setLogs when fetching logs
should capture the thrown error (e.g., catch (err)) and include its details in
the log message to aid diagnostics; update the handler around setLogs (the code
using setLogs and agentName in AgentDetailPanel) to append or interpolate
err.message (and optionally err.name or JSON.stringify(err)) into the
timestamped message and also consider console.error(err) for developer
visibility.
In `@desktop/src/apps/agents/constants.ts`:
- Around line 12-17: STATUS_STYLES is currently typed as Record<string,string>,
which hides missing/typoed keys for Agent.status; change its typing to be
exhaustive by declaring it as Record<Agent["status"], string> or use an explicit
const object with satisfies Record<Agent["status"], string> so the compiler
enforces all Agent.status values are present (update the STATUS_STYLES
declaration and imports so Agent type is available where STATUS_STYLES is
defined).
In `@desktop/src/apps/agents/types.ts`:
- Line 1: The shared types module is incorrectly coupled to a UI component by
importing AgentModel from ModelPickerFlow; extract the AgentModel type into a
neutral types module (e.g., create agent-model.ts exporting AgentModel), update
the ModelPickerFlow component to import AgentModel from that new module, and
change desktop/src/apps/agents/types.ts to import AgentModel from the new
neutral module instead of "`@/components/ModelPickerFlow`"; ensure the new module
exports the same type name and update any other imports (lines referenced around
60–61) to use the new module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 46eeea5b-b96b-4d09-bf9b-cfa4fd7cf1aa
📒 Files selected for processing (7)
desktop/src/apps/AgentsApp.tsxdesktop/src/apps/agents/AgentDetailPanel.tsxdesktop/src/apps/agents/AgentRow.tsxdesktop/src/apps/agents/ArchivedAgents.tsxdesktop/src/apps/agents/DeployWizard.tsxdesktop/src/apps/agents/constants.tsdesktop/src/apps/agents/types.ts
Comment on lines
+78
to
+81
| const dismissMigrationBanner = async () => { | ||
| await fetch(`/api/agents/${encodeURIComponent(agentName)}/dismiss-migration-banner`, { method: "POST" }); | ||
| onAgentUpdated(); | ||
| }; |
There was a problem hiding this comment.
Add error handling for the banner dismissal request.
The POST to dismiss the migration banner has no error handling. If the request fails, onAgentUpdated() is still called, which could lead to inconsistent state. Consider wrapping in a try/catch or checking res.ok.
🛡️ Proposed fix to add error handling
const dismissMigrationBanner = async () => {
- await fetch(`/api/agents/${encodeURIComponent(agentName)}/dismiss-migration-banner`, { method: "POST" });
- onAgentUpdated();
+ try {
+ const res = await fetch(`/api/agents/${encodeURIComponent(agentName)}/dismiss-migration-banner`, { method: "POST" });
+ if (!res.ok) {
+ console.error(`Failed to dismiss migration banner: ${res.status}`);
+ return;
+ }
+ onAgentUpdated();
+ } catch (err) {
+ console.error('Error dismissing migration banner:', err);
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const dismissMigrationBanner = async () => { | |
| await fetch(`/api/agents/${encodeURIComponent(agentName)}/dismiss-migration-banner`, { method: "POST" }); | |
| onAgentUpdated(); | |
| }; | |
| const dismissMigrationBanner = async () => { | |
| try { | |
| const res = await fetch(`/api/agents/${encodeURIComponent(agentName)}/dismiss-migration-banner`, { method: "POST" }); | |
| if (!res.ok) { | |
| console.error(`Failed to dismiss migration banner: ${res.status}`); | |
| return; | |
| } | |
| onAgentUpdated(); | |
| } catch (err) { | |
| console.error('Error dismissing migration banner:', err); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/agents/AgentDetailPanel.tsx` around lines 78 - 81, The
dismissMigrationBanner function lacks error handling so a failed POST still
triggers onAgentUpdated; wrap the fetch in a try/catch, await the response and
check res.ok, call onAgentUpdated only on success, and handle failures by
logging the error and/or surfacing a user-visible error state; update the
function named dismissMigrationBanner to perform the fetch -> if (!res.ok) throw
or handle error -> on success call onAgentUpdated, and catch any exceptions to
log/report them.
Comment on lines
+754
to
+756
| kv_cache_quant_k: kvCacheQuantK, | ||
| kv_cache_quant_v: kvCacheQuantV, | ||
| kv_cache_quant_boundary_layers: kvCacheQuantBoundaryLayers, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if DeployAgentRequest includes KV quant fields
# Search for kv_cache_quant in the DeployAgentRequest model
rg -nA 30 'class DeployAgentRequest' --type py | head -60
# Also check if there's a separate endpoint or middleware handling these fields
rg -n 'kv_cache_quant' --type pyRepository: jaylfc/taOS
Length of output: 12323
🏁 Script executed:
#!/bin/bash
# Inspect DeployAgentRequest model and deploy endpoint handler for kv_cache_quant_* handling and Pydantic extra behavior.
set -euo pipefail
# Show DeployAgentRequest definition + any Pydantic config (model_config / Config)
sed -n '150,260p' tinyagentos/routes/agents.py
# Show the /api/agents/deploy endpoint implementation around where body is parsed
rg -n "router\.post\\(\"/api/agents/deploy\"\\)" tinyagentos/routes/agents.py
sed -n '260,420p' tinyagentos/routes/agents.py
# Search for kv_cache_quant_* usage in deploy flow / agent creation
rg -n "kv_cache_quant_(k|v|boundary_layers)" tinyagentos/routes/agents.py tinyagentos -S
# Inspect config validation/migration for kv_cache_quant_* fields
rg -n "def validate_config|validate_config|migrat|kv_cache_quant_boundary_layers|kv_cache_quant_k|kv_cache_quant_v" tinyagentos/config.py
sed -n '150,270p' tinyagentos/config.pyRepository: jaylfc/taOS
Length of output: 21171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find any model_config / Config in agents.py affecting BaseModel extra handling
rg -n "model_config|class Config|extra\s*=" tinyagentos/routes/agents.py
# 2) Inspect deployer module for DeployRequest fields & how deploy_agent builds inference config
rg -n "class DeployRequest" tinyagentos/deployer.py tinyagentos/deployer/** 2>/dev/null || true
rg -n "kv_cache_quant_(k|v|boundary_layers)" tinyagentos/deployer.py tinyagentos/deployer 2>/dev/null || true
# 3) Search for use of kv_cache_quant_k/v/boundary_layers in inference/clients path
rg -n "kv_cache_quant_k|kv_cache_quant_v|kv_cache_quant_boundary_layers" tinyagentos/clients tinyagentos -S 2>/dev/null | head -200Repository: jaylfc/taOS
Length of output: 37
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show deployer file layout
ls -la tinyagentos | sed -n '1,200p'
find tinyagentos -maxdepth 2 -type f -name "deployer*" | sed -n '1,200p'
# Check Pydantic extra behavior (global and local)
rg -n "class .*Config|model_config|extra\s*=\s*\"(ignore|forbid|allow)\"|extra\s*=\s*Extra\.(ignore|forbid|allow)" tinyagentos | head -200 || true
# Find DeployRequest definition
rg -n "class DeployRequest" tinyagentos | head -200 || true
rg -n "DeployRequest\(" tinyagentos | head -200 || true
# Show deployer.py/deployer module around DeployRequest if present
DEPLOYER_FILE="$(rg -l "class DeployRequest" tinyagentos | head -1 || true)"
if [ -n "${DEPLOYER_FILE}" ]; then
echo "---- ${DEPLOYER_FILE} ----"
sed -n '1,240p' "${DEPLOYER_FILE}"
sed -n '240,520p' "${DEPLOYER_FILE}"
fi
# Search where kv_cache_quant_k/v/boundary_layers are consumed during deploy
rg -n "kv_cache_quant_k|kv_cache_quant_v|kv_cache_quant_boundary_layers" tinyagentos | head -200 || trueRepository: jaylfc/taOS
Length of output: 32102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect DeployRequest model in tinyagentos/deployer.py
rg -n "class DeployRequest" tinyagentos/deployer.py
LINE=$(rg -n "class DeployRequest" tinyagentos/deployer.py | head -1 | cut -d: -f1)
START=$((LINE-60))
END=$((LINE+140))
sed -n "${START},${END}p" tinyagentos/deployer.py
# Inspect Pydantic extra handling used by BaseModel in this project (look for ConfigDict/extra settings)
rg -n "extra\s*=\s*['\"](ignore|forbid|allow)['\"]|ConfigDict\\(.*extra|model_config\\s*=\\s*ConfigDict" tinyagentos | head -200 || true
# Search for actual consumption of kv_cache_quant_k/v/boundary_layers in inference/build path
rg -n "kv_cache_quant_(k|v|boundary_layers)" tinyagentos | head -200Repository: jaylfc/taOS
Length of output: 12283
KV cache quant UI selections aren’t persisted/used in /api/agents/deploy
desktop/src/apps/agents/DeployWizard.tsxsendskv_cache_quant_k,kv_cache_quant_v, andkv_cache_quant_boundary_layers.- Backend
tinyagentos/routes/agents.pyDeployAgentRequestdoes not define these fields, anddeploy_agent_endpointalso doesn’t copy them into the stored agent config passed tonormalize_agent. tinyagentos/config.py:normalize_agent()therefore defaults them tokv_cache_quant_k="fp16",kv_cache_quant_v="fp16", andkv_cache_quant_boundary_layers=0(plus there’s a TODO to thread these fields through the inference path once KV-quant support lands).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/agents/DeployWizard.tsx` around lines 754 - 756,
DeployWizard is sending kv_cache_quant_k, kv_cache_quant_v, and
kv_cache_quant_boundary_layers but DeployAgentRequest and deploy_agent_endpoint
aren’t persisting them; add these three fields to the DeployAgentRequest
Pydantic model in tinyagentos/routes/agents.py (matching types used in the
frontend), then update deploy_agent_endpoint to copy kv_cache_quant_k,
kv_cache_quant_v, and kv_cache_quant_boundary_layers into the agent config/dict
that you pass to normalize_agent (tinyagentos/config.py) so normalize_agent
receives and preserves these values (and ensure any type/field names match
normalize_agent’s expectations or extend normalize_agent to accept them).
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.
Modularity plan B6a (first half of the frontend god-file split). Decomposes the 2601-line
AgentsApp.tsxinto focused files underapps/agents/, following theapps/SettingsApp/panel convention:agents/types.ts— sharedAgent/DiskState/ArchivedAgent/Framework/Modeltypes.agents/constants.ts— COLORS / STATUS_STYLES / MEMORY_STEPS_MB.agents/AgentRow.tsx,agents/AgentDetailPanel.tsx,agents/DeployWizard.tsx(incl. MemoryWizardStep + MEMORY_TIER_INFO, co-located),agents/ArchivedAgents.tsx.Components moved verbatim (no markup/logic changes); the main
AgentsAppstays inAgentsApp.tsxand imports them. No circular imports. AgentsApp.tsx 2601 → 554.npm run build(tsc -b && vite build) passes with zero TypeScript errors.Summary by CodeRabbit
Release Notes