Skip to content

Conversation

@shuv1337
Copy link
Collaborator

@shuv1337 shuv1337 commented Dec 10, 2025

Summary

Changes

Removes:

  • subagents field from Agent schema and built-in agents
  • subagents config option for filtering agent invocations
  • filterSubagents function and runtime validation in Task tool
  • Subagent filtering from prompt tool resolution
  • Subagent filtering from TUI autocomplete suggestions
  • subagents-filter.test.ts test file
  • Subagents configuration documentation from agents.mdx

The SDK types will be regenerated automatically on the next build.

Context

This was originally from upstream PR anomalyco#4773 which added the ability to hide subagents from primary agents. The feature is not going to be merged upstream in its current form, so we're removing it from our fork.

Summary by CodeRabbit

  • Refactor

    • Removed public subagents configuration and related subagent filtering/access-control from agent surfaces.
  • New Features

    • In-session message search with highlighted results and navigation.
    • Live token usage display (input/output/reasoning) during streaming with a toggle.
    • Enhanced Bash tool output: ANSI-colored rendering, truncation with “view full output,” and full-view handler.
  • Documentation

    • Removed Subagents documentation and updated merged-PR listings.
  • Tests

    • Deleted subagents filtering test suite.

✏️ Tip: You can customize this high-level summary in your review settings.

This feature is not going to be merged upstream in its current form.

Removes:
- subagents field from Agent schema and built-in agents
- subagents config option
- filterSubagents function and runtime validation
- Subagent filtering from prompt tool resolution
- Subagent filtering from TUI autocomplete
- subagents-filter.test.ts test file
- Subagents documentation section from agents.mdx

The SDK types will be regenerated automatically on the next build.
@chatgpt-codex-connector
Copy link

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2025

Walkthrough

This PR removes the public subagents surface (schemas, tests, docs, runtime filters) and concurrently adds session search, live streaming token estimates, improved Bash/ANSI rendering with ghostty, subagent-sidebar navigation fixes, and several UI/context surface extensions.

Changes

Cohort / File(s) Summary
Subagents removal
packages/opencode/src/agent/agent.ts, packages/opencode/src/config/config.ts, packages/sdk/openapi.json, packages/opencode/test/subagents-filter.test.ts, packages/web/src/content/docs/agents.mdx
Deleted public subagents property from Agent schemas and SDK OpenAPI, removed related tests and documentation, and eliminated runtime/export of subagent filtering/merge logic.
Autocomplete UI
packages/opencode/src/cli/cmd/tui/component/prompt/autocomplete.tsx
Removed Wildcard import and subagent-based wildcard filtering from agent autocomplete; agents no longer filtered by caller subagents.
Task tool / prompt
packages/opencode/src/tool/task.ts, packages/opencode/src/session/prompt.ts
Removed filterSubagents helper and subagent-based runtime validation/description regeneration; TASK_DESCRIPTION is no longer augmented per-caller subagents.
Session search & TUI surface
packages/opencode/src/cli/cmd/tui/routes/session/index.tsx
Added in-session search (query, matches, navigation), showTokens toggle, search highlighting, showBashOutput handler, BashOutputView/SearchMatch types, Ghostty terminal renderable registration, and command wiring.
Sidebar navigation fix
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx
Changed navigation to use subagent session ID from part metadata and guarded navigation when missing (no longer uses parent part.sessionID).
Live token streaming
packages/opencode/src/session/processor.ts
Accumulates token deltas for reasoning and output during streaming and sets per-message token estimates via Token utilities.
Bash tool & ANSI rendering
packages/opencode/src/tool/bash.ts, packages/opencode/package.json
Forces color/terminal-related env vars for spawned bash processes and added ghostty-opentui dependency to enable ANSI/terminal rendering.
TUI Bash output viewer
packages/opencode/src/cli/cmd/tui/routes/session/index.tsx (renderer registration)
Integrates GhosttyTerminalRenderable for terminal buffers, adds truncation preview and click-to-view full output via showBashOutput plumbing.
Docs / plans / README
README.md, CONTEXT/PLAN-*.md
Removed merged PR row for configurable subagent visibility and added/updated internal plan documents for live tokens, bash viewer, subagent navigation, and search restoration.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing extra attention:
    • packages/opencode/src/cli/cmd/tui/routes/session/index.tsx (large, multiple public/context additions: search, tokens, bash renderer).
    • packages/opencode/src/session/processor.ts (token accumulation and message estimate wiring).
    • packages/opencode/src/tool/task.ts and related removals to ensure no remaining references to subagents.
    • Ghostty integration and package.json dependency addition for packaging/runtime implications.

Possibly related PRs

Poem

🐰 I nibble code beneath the moonlit tree,
Subagents folded, hidden quietly.
Searches sparkle, tokens count and hum,
Bash now colors — bright terminal drum.
Hop, patch, and carrot — job well done! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately describes the main change: reverting/removing the subagent restrictions feature from PR anomalyco#4773. It directly corresponds to the primary changeset objective.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch revert-pr-4773-subagent-restrictions

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/opencode/src/agent/agent.ts (1)

171-228: Intentionally ignoring subagents from config while keeping it out of options

Destructuring subagents: _subagents and then not using it is a reasonable way to tolerate legacy subagents keys in user config while ensuring they neither affect Agent.Info nor leak into options. That’s consistent with dropping subagents from the public agent surface.

If Config’s agent schema has definitively removed subagents already, this alias becomes redundant; you could either drop it or add a brief comment (e.g. “legacy field intentionally ignored”) for future maintainers.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5630efc and d273a44.

⛔ Files ignored due to path filters (1)
  • packages/sdk/js/src/v2/gen/types.gen.ts is excluded by !**/gen/**
📒 Files selected for processing (8)
  • packages/opencode/src/agent/agent.ts (1 hunks)
  • packages/opencode/src/cli/cmd/tui/component/prompt/autocomplete.tsx (0 hunks)
  • packages/opencode/src/config/config.ts (0 hunks)
  • packages/opencode/src/session/prompt.ts (1 hunks)
  • packages/opencode/src/tool/task.ts (0 hunks)
  • packages/opencode/test/subagents-filter.test.ts (0 hunks)
  • packages/sdk/openapi.json (1 hunks)
  • packages/web/src/content/docs/agents.mdx (0 hunks)
💤 Files with no reviewable changes (5)
  • packages/opencode/src/cli/cmd/tui/component/prompt/autocomplete.tsx
  • packages/opencode/test/subagents-filter.test.ts
  • packages/opencode/src/config/config.ts
  • packages/opencode/src/tool/task.ts
  • packages/web/src/content/docs/agents.mdx
🧰 Additional context used
📓 Path-based instructions (3)
packages/opencode/**/*.ts

📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)

packages/opencode/**/*.ts: Use Bun with TypeScript ESM modules as the runtime
Organize file structure using namespace-based organization (e.g., Tool.define(), Session.create())
Validate all inputs with Zod schemas
Use Log.create({ service: "name" }) pattern for logging
Use Storage namespace for persistence operations

Files:

  • packages/opencode/src/session/prompt.ts
  • packages/opencode/src/agent/agent.ts
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/session/prompt.ts
  • packages/opencode/src/agent/agent.ts
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/session/prompt.ts
  • packages/opencode/src/agent/agent.ts
🔇 Additional comments (3)
packages/opencode/src/session/prompt.ts (2)

343-354: Remaining subagent_type references are appropriate.

The subagent_type parameter usage here is part of the task tool's invocation mechanism, which allows one agent to invoke another. This is distinct from the subagent restrictions feature being removed (which would have filtered/validated which subagents could be invoked). The removal correctly eliminates the filtering logic while preserving the underlying invocation capability.


50-50: Remove unused TASK_DESCRIPTION from the import statement.

The import of TASK_DESCRIPTION on line 50 is now unused in this file and should be removed along with filterSubagents. The line should be:

import { TaskTool } from "@/tool/task"

Verification confirms that filterSubagents has been completely removed from the codebase with no dangling references, but the import statement still includes TASK_DESCRIPTION which is not utilized anywhere in prompt.ts.

⛔ Skipped due to learnings
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 : Implement tools with the `Tool.Info` interface including an `execute()` method
packages/sdk/openapi.json (1)

8704-8799: Agent required fields now correctly omit subagents

The Agent schema’s required array matches the remaining properties and cleanly drops subagents, aligning the OpenAPI surface with the reverted subagent restrictions. No structural issues here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
packages/opencode/src/tool/bash.ts (1)

225-238: Color/TTY env forcing is localized but overrides user preferences

The added env block correctly scopes color/TTY hints to the bash tool subprocess, which is good for isolating behavior. Note that this force-enables color (and explicitly unsets NO_COLOR) even if the user has configured the opposite in their environment. If you want to preserve user preferences while still improving defaults, consider only setting these keys when they’re not already defined in process.env.

packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx (1)

243-261: Subagent navigation now targets correct session; consider UX for missing IDs

Deriving subagentSessionId from part.state.metadata and guarding route.navigate on its presence fixes the earlier bug where clicks jumped back to the parent session. The status-based metadata selection is reasonable given the different state shapes.

When subagentSessionId is absent (e.g., very early in task execution or older sessions), the row still appears interactive but clicks no-op. Consider a subtle disabled style or tooltip in that case so users aren’t left wondering why nothing happens.

CONTEXT/PLAN-4865-restore-subagent-navigation-2025-12-09.md (1)

78-191: Subagent navigation plan matches code; consider adjusting the “visual feedback” checklist

The root-cause analysis and proposed fix (deriving subagentSessionId from tool metadata and guarding navigation) match the implemented change in sidebar.tsx. One minor nit: Phase 1 marks “Add visual feedback when subagent session ID is not available” as completed, but the current sidebar code only no-ops on click without a visual cue. Either implementing a disabled style/tooltip or unchecking that item would bring the doc fully in line with behavior.

packages/opencode/src/cli/cmd/tui/routes/session/index.tsx (1)

184-196: Consider using computed index directly for clarity.

While SolidJS signal updates are synchronous, the current pattern reads currentMatchIndex() after setting it. Using the computed value directly is clearer and avoids potential confusion:

 function handleNextMatch() {
   const m = matches()
   if (m.length === 0) return
-  setCurrentMatchIndex((prev) => (prev + 1) % m.length)
-  scrollToMatch(currentMatchIndex())
+  const newIndex = (currentMatchIndex() + 1) % m.length
+  setCurrentMatchIndex(newIndex)
+  scrollToMatch(newIndex)
 }

 function handlePrevMatch() {
   const m = matches()
   if (m.length === 0) return
-  setCurrentMatchIndex((prev) => (prev - 1 + m.length) % m.length)
-  scrollToMatch(currentMatchIndex())
+  const newIndex = (currentMatchIndex() - 1 + m.length) % m.length
+  setCurrentMatchIndex(newIndex)
+  scrollToMatch(newIndex)
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d273a44 and 7c31f5b.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (10)
  • CONTEXT/PLAN-4709-restore-live-token-usage-2025-12-09.md (1 hunks)
  • CONTEXT/PLAN-4791-restore-bash-viewer-ansi-2025-12-09.md (1 hunks)
  • CONTEXT/PLAN-4865-restore-subagent-navigation-2025-12-09.md (1 hunks)
  • CONTEXT/PLAN-4898-restore-search-in-messages-2025-12-09.md (1 hunks)
  • README.md (1 hunks)
  • packages/opencode/package.json (1 hunks)
  • packages/opencode/src/cli/cmd/tui/routes/session/index.tsx (20 hunks)
  • packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx (1 hunks)
  • packages/opencode/src/session/processor.ts (4 hunks)
  • packages/opencode/src/tool/bash.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/src/tool/bash.ts
  • packages/opencode/src/session/processor.ts
  • packages/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.tsx
  • packages/opencode/src/tool/bash.ts
  • packages/opencode/src/session/processor.ts
  • packages/opencode/src/cli/cmd/tui/routes/session/index.tsx
packages/opencode/**/*.ts

📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)

packages/opencode/**/*.ts: Use Bun with TypeScript ESM modules as the runtime
Organize file structure using namespace-based organization (e.g., Tool.define(), Session.create())
Validate all inputs with Zod schemas
Use Log.create({ service: "name" }) pattern for logging
Use Storage namespace for persistence operations

Files:

  • packages/opencode/src/tool/bash.ts
  • packages/opencode/src/session/processor.ts
🧠 Learnings (3)
📚 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/index.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 function declarations and splitProps for SolidJS component props

Applied to files:

  • packages/opencode/src/cli/cmd/tui/routes/session/index.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/index.tsx
🧬 Code graph analysis (1)
packages/opencode/src/cli/cmd/tui/routes/session/sidebar.tsx (1)
packages/opencode/src/session/prompt.ts (1)
  • metadata (363-372)
🪛 LanguageTool
CONTEXT/PLAN-4709-restore-live-token-usage-2025-12-09.md

[grammar] ~205-~205: Ensure spelling is correct
Context: ...onsider throttling updates (e.g., every 100ms or 100 chars). 3. Context Limit: D...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

CONTEXT/PLAN-4898-restore-search-in-messages-2025-12-09.md

[uncategorized] ~81-~81: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...arkdownSearchHighlighter` component for markdown with code block handling - [x] Add rege...

(MARKDOWN_NNP)

CONTEXT/PLAN-4791-restore-bash-viewer-ansi-2025-12-09.md

[style] ~184-~184: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...s in the TUI may impact performance for very large outputs. The 20-line preview helps miti...

(EN_WEAK_ADJECTIVE)


[grammar] ~188-~188: Use a hyphen to join words.
Context: ...ing. 4. Platform Differences: Color forcing env vars may behave differently ...

(QB_NEW_EN_HYPHEN)

🪛 markdownlint-cli2 (0.18.1)
CONTEXT/PLAN-4709-restore-live-token-usage-2025-12-09.md

4-4: Bare URL used

(MD034, no-bare-urls)


175-175: Bare URL used

(MD034, no-bare-urls)

CONTEXT/PLAN-4898-restore-search-in-messages-2025-12-09.md

4-4: Bare URL used

(MD034, no-bare-urls)


119-119: Bare URL used

(MD034, no-bare-urls)

CONTEXT/PLAN-4791-restore-bash-viewer-ansi-2025-12-09.md

24-24: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🔇 Additional comments (17)
README.md (1)

15-24: README metadata reflects reverted subagent feature correctly

The removal of the subagent-related PR from the table and the updated _Last updated_ date are consistent with reverting that feature. No further changes needed here.

packages/opencode/package.json (1)

48-90: ghostty-opentui dependency addition aligns with new TUI features

Adding ghostty-opentui@1.3.6 matches the new Ghostty-based bash/ANSI rendering paths. Please just confirm this version is compatible with your current @opentui/* versions and that the workspace lockfile (if any) is updated accordingly.

CONTEXT/PLAN-4898-restore-search-in-messages-2025-12-09.md (1)

1-136: Search-in-messages plan is clear and matches the implemented feature set

The phased breakdown (state/types, handlers, highlighters, context wiring, and validation criteria) is coherent and aligns with how the TUI search feature is typically structured. I wouldn’t change anything here; the doc is a solid reference for future maintenance.

CONTEXT/PLAN-4709-restore-live-token-usage-2025-12-09.md (1)

1-208: Live token usage plan aligns with processor/UI changes and flags remaining TODOs

This plan cleanly describes the streaming token pipeline (processor updates, TUI state, toggle command, display format) and correctly leaves finish-step / contextLimit work unchecked. It’s a good, accurate design note to keep alongside the code.

packages/opencode/src/session/processor.ts (1)

15-15: Token estimation wiring is correct; consider syncing estimates during streaming for live display

The reasoningTotal / textTotal accumulators and use of Token.toTokenEstimate(...) for reasoningEstimate and outputEstimate are correctly implemented and scoped to each process run.

The TUI reads these estimates from persisted session state via props.message. Currently, these in-memory updates (lines 93, 322) are not persisted until finish-step completes (line 276), so clients will only see final token estimates after streaming ends, not during streaming. To enable true "live" token display while streaming, consider calling Session.updateMessage(input.assistantMessage) (ideally throttled) inside the reasoning-delta and text-delta branches to sync estimates to session state as they accumulate.

CONTEXT/PLAN-4791-restore-bash-viewer-ansi-2025-12-09.md (1)

1-188: Well-structured implementation plan.

The document provides clear phase breakdowns, status tracking, validation criteria, and risk considerations. The approach for forcing color output via environment variables is well-documented.

Minor linter suggestions (hyphenation of "color-forcing", "very large" style) are non-blocking for internal documentation.

packages/opencode/src/cli/cmd/tui/routes/session/index.tsx (11)

68-70: LGTM! Ghostty terminal component properly registered.

The GhosttyTerminalRenderable import and registration via extend follows the correct pattern for adding custom components to opentui.


106-116: Clean type definitions.

The SearchMatch type captures all necessary information for navigating search results. BashOutputView correctly uses an accessor function () => string for lazy output evaluation.


274-285: LGTM! Keyboard handling follows expected patterns.

ESC to close the bash viewer and Ctrl+F for search are standard shortcuts. The early return when bashOutput() is active correctly prevents event propagation.


334-343: Search command registration follows existing patterns.

The "Search messages" command is properly registered with the command dialog, including keybind and category.


580-592: Token visibility toggle properly persisted.

The toggle follows the same pattern as other session toggles, persisting state to kv store.


1098-1129: LGTM! Search input properly integrated.

The search input receives all necessary callbacks and state is properly cleaned up on exit. The matchInfo prop provides current/total counts for the UI.


1273-1329: Token display implementation is well-designed.

The token calculations properly combine actual values with estimates using fallback patterns. The ~ prefix clearly indicates these are estimates, and toLocaleString() improves readability for large numbers.


1565-1601: LGTM! Bash tool properly integrates ghostty terminal rendering.

The output memo correctly preserves ANSI codes for rendering. The limit={20} prop handles truncation, and the "Click to see full output" interaction correctly passes the output accessor to showBashOutput.


1919-1922: LGTM! Standard regex escape implementation.

The function correctly escapes all regex metacharacters, preventing injection when building patterns from user search queries.


958-964: Context provider properly extended with new features.

All new fields (showTokens, searchQuery, currentMatchIndex, matches, showBashOutput) are correctly wired to the context provider, matching the type definition.


1089-1095: Remove or verify PageUp/PageDown scrolling functionality in bash viewer.

The help text at line 1094 states "PageUp/PageDown to scroll", but the keyboard handler (lines 274-280) only processes the ESC key when bashOutput() is active and returns immediately for all other keys. No keybind configuration for PageUp/PageDown is present in the codebase. Either confirm that ghostty-opentui handles scrolling natively and update the help text to clarify this, or implement explicit keyboard handlers for PageUp/PageDown scrolling.

Copy link

Copilot AI commented Dec 10, 2025

@kcrommett I've opened a new pull request, #106, to work on those changes. Once the pull request is ready, I'll request review from you.

shuv1337 and others added 2 commits December 10, 2025 00:28
* Initial plan

* fix: replace strikethrough with ANSI highlighting for search results

Co-authored-by: kcrommett <523952+kcrommett@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kcrommett <523952+kcrommett@users.noreply.github.com>
@shuv1337 shuv1337 merged commit 4690583 into integration Dec 10, 2025
1 of 3 checks passed
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/opencode/src/cli/cmd/tui/routes/session/index.tsx (2)

91-203: Search context surface and navigation logic are well-structured; consider tightening match scrolling

The additional context fields (showTokens, searchQuery, currentMatchIndex, matches, showBashOutput) and the SearchMatch/BashOutputView types are cohesive and make downstream usage clear. The matches memo correctly short-circuits when searchQuery is empty and performs a simple, case-insensitive scan over text parts.

One small clarity improvement: in handleNextMatch / handlePrevMatch, you can compute the next index once and pass it directly to scrollToMatch instead of reading currentMatchIndex() after the setter, which avoids any dependence on signal timing semantics and makes the intent explicit:

-  function handleNextMatch() {
-    const m = matches()
-    if (m.length === 0) return
-    setCurrentMatchIndex((prev) => (prev + 1) % m.length)
-    scrollToMatch(currentMatchIndex())
-  }
+  function handleNextMatch() {
+    const m = matches()
+    if (m.length === 0) return
+    setCurrentMatchIndex((prev) => {
+      const next = (prev + 1) % m.length
+      scrollToMatch(next)
+      return next
+    })
+  }

(and similarly for handlePrevMatch).


1376-1388: Search highlighting via ANSI is safe; consider optional multi-part awareness

Replacing the previous markdown-strikethrough approach with ANSI-based highlighting in TextPart is a nice improvement for actual visibility. Escaping the query via escapeRegex before building the RegExp prevents regex injection, and limiting the substitution to props.part.text.trim() keeps behavior identical when no query is active.

Two minor, optional enhancements you might consider later:

  • If you want search to feel fully consistent across roles, mirror this highlighting for user text as well, not just assistant TextParts.
  • If the markdown renderer ever treats ANSI sequences oddly inside filetype="markdown" code blocks, you could add an explicit ansi-aware path (similar to the ghostty-terminal usage) or strip ANSI for non-TTY exports, but that’s more of a UX polish than a correctness issue.

Overall, the current implementation is sound.

Also applies to: 1398-1398, 1921-1923

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c31f5b and c2ed115.

📒 Files selected for processing (1)
  • packages/opencode/src/cli/cmd/tui/routes/session/index.tsx (20 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/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/index.tsx
🧠 Learnings (3)
📚 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/index.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 function declarations and splitProps for SolidJS component props

Applied to files:

  • packages/opencode/src/cli/cmd/tui/routes/session/index.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/index.tsx
🧬 Code graph analysis (1)
packages/opencode/src/cli/cmd/tui/routes/session/index.tsx (1)
packages/opencode/src/cli/cmd/tui/component/prompt/search.tsx (1)
  • SearchInput (35-230)
🔇 Additional comments (5)
packages/opencode/src/cli/cmd/tui/routes/session/index.tsx (5)

28-71: SearchInput and Ghostty terminal integration look consistent

The wiring for SearchInput and GhosttyTerminalRenderable via extend({ "ghostty-terminal": GhosttyTerminalRenderable }) is straightforward and matches existing patterns for OpenTUI extensions; no issues from a composition/registration standpoint.


271-285: Keyboard handling for bash output and search mode is sensible

Short-circuiting all global key handling while bashOutput() is active (except ESC to close) is a good safety measure and prevents accidental session actions while in the fullscreen terminal view. Adding ctrl+f here to enter searchMode lines up with the SearchInput keybinding behavior and doesn’t appear to conflict with existing permission-handling logic.


333-343: Command palette entries for search and token toggling are well-integrated

The new "Search messages" command cleanly flips searchMode and clears the dialog, relying on SearchInput’s own mount-time focus, which is simple and predictable. The "Show/Hide tokens" entry correctly persists state via kv.set("show_tokens", next) and reuses the existing showTokens signal exposed in context; naming and behavior are consistent with the other toggles.

Also applies to: 580-592


1234-1236: Token display wiring is correct and non-intrusive

Using ctx.showTokens() to gate both the user sentEstimate display and the assistant IN/OUT/reasoning token summary is a clean way to add live metrics without cluttering the default UI. The token computations (sentEstimate + contextEstimate, tokens.output ?? outputEstimate, tokens.reasoning ?? reasoningEstimate) are straightforward and fall back gracefully when estimates or finalized counts are missing.

Also applies to: 1257-1281, 1324-1329


1570-1597: Bash tool ghostty rendering and truncation UX are well thought out

The Bash tool’s render function now:

  • Derives output from props.metadata.output?.trim() ?? "",
  • Uses <ghostty-terminal ansi={output()} limit={20} /> for a compact inline view, and
  • Detects truncation via lines().length > 20, offering a clickable “see full output” that calls showBashOutput.

This fits nicely with the fullscreen bashOutput view and keeps long command output from overwhelming the conversation while still being easily accessible. No issues spotted with the logic or context usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants