Sidebar fix #97
Sidebar fix #97
Conversation
WalkthroughConditional "Go to parent session" enabling was added; a collapsible Subagents section with grouped subagent items, status indicators, and navigation was introduced to the sidebar; two OpenAPI JSON files were reformatted (whitespace/line breaks only). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx (2)
50-60: Consider adding type validation for subagent_type.The casting of
input?.subagent_type as stringon line 54 assumes the value is a string without validation. While line 55 guards against falsy values, it doesn't verify the type. Consider using a type guard or runtime check to ensure type safety.Example improvement:
const subagentGroups = createMemo(() => { const groups = new Map<string, ToolPart[]>() for (const part of taskToolParts()) { const input = part.state.input as Record<string, unknown> const agentName = input?.subagent_type as string - if (!agentName) continue + if (!agentName || typeof agentName !== 'string') continue if (!groups.has(agentName)) groups.set(agentName, []) groups.get(agentName)!.push(part) } return Array.from(groups.entries()) })
245-255: Consider adding visual feedback for clickable subagent items.The code navigates to a subagent's session when clicked (lines 253-255), but only if a
sessionIdexists. There's no visual indication (e.g., cursor style, hover effect, or color change) to show which items are clickable versus non-clickable. This could lead to user confusion.Consider adding a conditional style or cursor indicator:
<box flexDirection="row" gap={1} paddingLeft={2} onMouseDown={() => { if (sessionId) route.navigate({ type: "session", sessionID: sessionId }) }} style={sessionId ? { cursor: "pointer" } : undefined} >Or use a color change to indicate clickability:
<text fg={isActive() ? theme.text : sessionId ? theme.textMuted : theme.textDisabled} wrapMode="word"> {description} </text>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/opencode/src/cli/cmd/tui/routes/session/index.tsx(1 hunks)packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.{ts,tsx,js,jsx}: Use relative imports for local modules, with named imports preferred
Use camelCase for variable and function names
Use PascalCase for classes and namespaces
Files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsxpackages/opencode/src/cli/cmd/tui/routes/session/index.tsx
packages/opencode/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.{ts,tsx}: Use Zod schemas for validation and TypeScript interfaces for structure
Use Result patterns for error handling; avoid throwing exceptions in tools
Files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsxpackages/opencode/src/cli/cmd/tui/routes/session/index.tsx
🧠 Learnings (5)
📚 Learning: 2025-12-02T19:05:12.128Z
Learnt from: CR
Repo: Latitudes-Dev/shuvcode PR: 0
File: packages/desktop/AGENTS.md:0-0
Timestamp: 2025-12-02T19:05:12.128Z
Learning: Applies to packages/desktop/**/*.{ts,tsx} : Use function declarations and splitProps for SolidJS component props
Applied to files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
📚 Learning: 2025-12-02T19:05:23.634Z
Learnt from: CR
Repo: Latitudes-Dev/shuvcode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-12-02T19:05:23.634Z
Learning: Applies to packages/opencode/**/tools/**/*.ts : Pass `sessionID` in tool context and use `App.provide()` for dependency injection
Applied to files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
📚 Learning: 2025-12-02T19:05:23.634Z
Learnt from: CR
Repo: Latitudes-Dev/shuvcode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-12-02T19:05:23.634Z
Learning: Applies to packages/opencode/**/*.ts : Organize file structure using namespace-based organization (e.g., `Tool.define()`, `Session.create()`)
Applied to files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
📚 Learning: 2025-12-02T19:05:23.634Z
Learnt from: CR
Repo: Latitudes-Dev/shuvcode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-12-02T19:05:23.634Z
Learning: Go TUI communicates with TypeScript server via stainless SDK. When adding/modifying server endpoints in `packages/opencode/src/server/server.ts`, ask the user to generate a new client SDK before proceeding with client-side changes
Applied to files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
📚 Learning: 2025-12-02T19:05:12.128Z
Learnt from: CR
Repo: Latitudes-Dev/shuvcode PR: 0
File: packages/desktop/AGENTS.md:0-0
Timestamp: 2025-12-02T19:05:12.128Z
Learning: Applies to packages/desktop/**/*.{ts,tsx} : Use SolidJS with TypeScript as the primary framework
Applied to files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
🧬 Code graph analysis (1)
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx (2)
packages/opencode/src/session/message-v2.ts (3)
parts(549-557)ToolPart(272-280)ToolPart(281-281)packages/sdk/js/src/v2/gen/types.gen.ts (1)
ToolPart(304-315)
🔇 Additional comments (6)
packages/opencode/src/cli/cmd/tui/routes/session/index.tsx (1)
765-765: LGTM! The conditional disabling correctly reflects navigation capability.The change properly gates the menu item based on whether a parent session exists, aligning with the onSelect handler's navigation logic. The optional chaining safely handles the case where session() might be undefined.
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx (5)
2-2: LGTM! Imports are correctly added for new functionality.The new imports (
createSignal,onCleanup,useRoute,ToolPart) are all properly used in the implementation below.Also applies to: 5-5, 8-8
14-14: LGTM! Route hook correctly initialized for navigation.The
routeobject is properly used later for navigating to subagent sessions.
29-35: LGTM! Spinner mechanism is properly implemented with cleanup.The interval-based spinner animation is correctly set up with
onCleanupto prevent memory leaks. The 100ms refresh rate provides smooth animation.
40-48: LGTM! Task tool parts aggregation is correctly implemented.The memo efficiently collects all task-related tool parts from messages, maintaining proper type safety with the
ToolPart[]annotation.
375-377: LGTM! Status bar labels updated.The branding labels have been updated from "Open" → "shuv" and "Code" → "code", maintaining consistency with the product naming.
|
@kcrommett I've opened a new pull request, #98, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
The fix has been applied. The changes made:
The |
Co-authored-by: kcrommett <kcrommett@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx (2)
29-36: Consider scoping the spinner interval to when Subagents are actually neededThe global
setIntervaldrivingspinnerIndexworks and is correctly cleaned up viaonCleanup. If you ever want to trim background work further, you could wrap this in acreateEffectthat only starts the timer when there are subagent entries (and/or active ones), and tears it down when none are present. This is purely an optimization; current behavior is acceptable.
40-60: Strengthen typing for task-tool input to avoid repeated casts
taskToolParts/subagentGroupscurrently accesspart.state.inputviaas Record<string, unknown>and then castsubagent_typetostring. For maintainability and type-safety, consider introducing a small interface (or reusing a Zod schema) for thetasktool input shape and narrowing to it once, e.g. aTaskToolInputtype withsubagent_type/descriptionfields. That would let you avoid genericRecord<string, unknown>casts and catch input-shape drift at compile time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx(4 hunks)packages/sdk/js/openapi.json(182 hunks)packages/sdk/openapi.json(182 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/sdk/js/openapi.json
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.{ts,tsx,js,jsx}: Use relative imports for local modules, with named imports preferred
Use camelCase for variable and function names
Use PascalCase for classes and namespaces
Files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
packages/opencode/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.{ts,tsx}: Use Zod schemas for validation and TypeScript interfaces for structure
Use Result patterns for error handling; avoid throwing exceptions in tools
Files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
🧠 Learnings (5)
📚 Learning: 2025-12-02T19:05:12.128Z
Learnt from: CR
Repo: Latitudes-Dev/shuvcode PR: 0
File: packages/desktop/AGENTS.md:0-0
Timestamp: 2025-12-02T19:05:12.128Z
Learning: Applies to packages/desktop/**/*.{ts,tsx} : Use function declarations and splitProps for SolidJS component props
Applied to files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
📚 Learning: 2025-12-02T19:05:23.634Z
Learnt from: CR
Repo: Latitudes-Dev/shuvcode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-12-02T19:05:23.634Z
Learning: Applies to packages/opencode/**/tools/**/*.ts : Pass `sessionID` in tool context and use `App.provide()` for dependency injection
Applied to files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
📚 Learning: 2025-12-02T19:05:23.634Z
Learnt from: CR
Repo: Latitudes-Dev/shuvcode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-12-02T19:05:23.634Z
Learning: Applies to packages/opencode/**/*.ts : Organize file structure using namespace-based organization (e.g., `Tool.define()`, `Session.create()`)
Applied to files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
📚 Learning: 2025-12-02T19:05:23.634Z
Learnt from: CR
Repo: Latitudes-Dev/shuvcode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-12-02T19:05:23.634Z
Learning: Go TUI communicates with TypeScript server via stainless SDK. When adding/modifying server endpoints in `packages/opencode/src/server/server.ts`, ask the user to generate a new client SDK before proceeding with client-side changes
Applied to files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
📚 Learning: 2025-12-02T19:05:12.128Z
Learnt from: CR
Repo: Latitudes-Dev/shuvcode PR: 0
File: packages/desktop/AGENTS.md:0-0
Timestamp: 2025-12-02T19:05:12.128Z
Learning: Applies to packages/desktop/**/*.{ts,tsx} : Use SolidJS with TypeScript as the primary framework
Applied to files:
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
🔇 Additional comments (6)
packages/sdk/openapi.json (3)
332-335: Formatting reflow looks good.Multi-line required arrays and enums are fine; no semantic change detected across similar blocks.
803-806: Quick spot-checks pass.
- Reflowed required/enum blocks preserve keys and values.
- Use of exclusiveMinimum/maximum is JSON Schema 2020-12 compliant in OAS 3.1.
No action needed.Also applies to: 1698-1704, 1892-1896, 2105-2109, 2346-2349, 2561-2567, 2684-2688, 3191-3193, 3202-3204, 3235-3238, 332-335
26-31: JSON is valid and code samples are correctly formatted.No issues were found. The JSON file parses successfully, and the code samples use correct JSON string escaping. The pattern
\nin JSON strings represents a newline character, not a missing quote. The samples likefrom \"@opencode-ai/sdk\n\n...are properly formed—the closing quote is present, followed by the escape sequence for a newline. The suggested fix would incorrectly insert literal backslashes instead of newline characters.Likely an incorrect or invalid review comment.
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx (3)
2-27: Imports, route usage, and expandedsubagentsstate look consistent with existing patternsThe added SolidJS imports,
useRoutehook wiring,ToolParttyping, and the newexpanded.subagentsflag integrate cleanly with the existing sidebar structure. No issues from a correctness or style standpoint.
209-270: Subagents grouping, status display, and navigation behavior look solidThe new Subagents section correctly:
- Groups task-tool parts by
subagent_type.- Derives group/row status (running, pending, error, idle) and maps them to reasonable glyphs and colors.
- Navigates to the appropriate session via
route.navigate({ type: "session", sessionID: part.sessionID })on mouse down.The collapse/expand logic mirrors existing MCP/LSP/Todo/Diff patterns, so UX is consistent. No changes requested here.
372-375: Footer label rename to “shuv code” is consistent and non-breakingThe updated footer text and styling (
shuv+code) hook into existing theme colors and leave the version display intact. No functional impact beyond the intended branding tweak.
packages/sdk/openapi.json
Outdated
| "tags": [ | ||
| "Session" | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's check the file exists and get basic info
echo "=== File info ==="
wc -l packages/sdk/openapi.json
echo ""
# Check lines 913-915 and 1113-1115 for context
echo "=== Context around lines 913-915 ==="
sed -n '910,920p' packages/sdk/openapi.json
echo ""
echo "=== Context around lines 1113-1115 ==="
sed -n '1110,1120p' packages/sdk/openapi.json
echo ""
# Run the provided shell script
echo "=== Session operations without tags ==="
jq -r '
.paths
| to_entries[]
| select(.key|test("^/session"))
| .value
| to_entries[]
| select(.value.operationId and (.value.operationId|startswith("session.")) and ((.value|has("tags"))|not))
| .value.operationId
' packages/sdk/openapi.json
echo ""
# Let's also see all session operations for comparison
echo "=== All session operations ==="
jq -r '
.paths
| to_entries[]
| select(.key|test("^/session"))
| .value
| to_entries[]
| select(.value.operationId and (.value.operationId|startswith("session.")))
| "\(.value.operationId): has_tags=\((.value|has("tags")))"
' packages/sdk/openapi.jsonRepository: Latitudes-Dev/shuvcode
Length of output: 1961
Add "Session" tag to all session operations for consistent documentation grouping.
Only 2 of 25 session operations (session.get and session.children) currently have tags. The remaining 23 operations (session.list, session.create, session.status, session.delete, session.update, session.todo, session.init, session.fork, session.abort, session.share, session.unshare, session.diff, session.summarize, session.messages, session.prompt, session.message, session.prompt_async, session.command, session.shell, session.revert, session.unrevert, and others) are missing the "Session" tag. Ensure all /session* operations are tagged consistently for uniform documentation organization.
🤖 Prompt for AI Agents
In packages/sdk/openapi.json around lines 913-915, the OpenAPI operations for
session endpoints are inconsistently tagged; update every operation under paths
that start with /session (including session.list, session.create,
session.status, session.delete, session.update, session.todo, session.init,
session.fork, session.abort, session.share, session.unshare, session.diff,
session.summarize, session.messages, session.prompt, session.message,
session.prompt_async, session.command, session.shell, session.revert,
session.unrevert, etc.) so each operation object contains "tags": ["Session"];
search all /session* path entries and add or replace their operation-level
"tags" arrays with ["Session"] to ensure uniform documentation grouping.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.