feat(tui): Add split worker/reviewer output with Tab navigation and focus management#272
Open
shawnsw wants to merge 18 commits intosubsy:mainfrom
Open
feat(tui): Add split worker/reviewer output with Tab navigation and focus management#272shawnsw wants to merge 18 commits intosubsy:mainfrom
shawnsw wants to merge 18 commits intosubsy:mainfrom
Conversation
Contributor
WalkthroughAdds an optional reviewer agent across config, engine, templates, CLI, and TUI: reviewer prompt/template resolution, review-stage execution after worker runs, review result handling (pass/fail/block), split worker/reviewer outputs and focus, Git-init prompts, and updated conflict-resolution callbacks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TUI
participant ExecutionEngine
participant TemplateEngine
participant WorkerAgent as Worker Agent
participant ReviewerAgent as Reviewer Agent
User->>TUI: Start task/run
TUI->>ExecutionEngine: startTask(task)
ExecutionEngine->>TemplateEngine: load worker prompt
TemplateEngine-->>ExecutionEngine: worker prompt + source
ExecutionEngine->>WorkerAgent: execute worker with prompt
WorkerAgent-->>ExecutionEngine: worker output + status
alt review enabled
ExecutionEngine->>TemplateEngine: load review template (task, worker output)
TemplateEngine-->>ExecutionEngine: review prompt + source
ExecutionEngine->>ReviewerAgent: execute reviewer with review prompt
ReviewerAgent-->>ExecutionEngine: review result (pass/fail) + output
alt review passed
ExecutionEngine->>ExecutionEngine: mark iteration completed
else review failed
ExecutionEngine->>ExecutionEngine: set taskBlocked, mark issues
end
else no review
ExecutionEngine->>ExecutionEngine: mark iteration completed
end
ExecutionEngine-->>TUI: emit iteration result (includes review metadata and combined outputs)
TUI->>User: render split worker/reviewer output and controls
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![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: 8
🤖 Fix all issues with AI agents
In `@src/config/index.ts`:
- Around line 438-462: getAgentConfigByName currently returns a bare
AgentPluginConfig when registry.hasPlugin(agentName), losing top-level
shorthands (command, timeout, agentOptions); update it to resolve and reuse the
full agent config (merging top-level shorthands) instead of returning an empty
options object — e.g., when registry.hasPlugin(agentName) is true, look up the
resolved agent config (reuse the same resolution used for the primary agent or
merge storedConfig top-level shorthand fields into the constructed config),
ensuring command, timeout and agentOptions are preserved for the returned
AgentPluginConfig; modify getAgentConfigByName and its registry check
(getAgentRegistry(), registry.hasPlugin(agentName)) to perform this merge.
In `@src/engine/index.ts`:
- Around line 1373-1383: When a review fails the code blocks the task but never
marks the forced task as processed, causing worker mode to re-run it; inside the
if (!reviewPassed) block (where taskBlocked is set,
this.skippedTasks.add(task.id) is called and
this.tracker!.updateTaskStatus(task.id, 'blocked') is awaited) set the forced
task processed flag (e.g. this.forcedTaskProcessed = true or the local
forcedTaskProcessed variable used by the engine) so the engine recognizes the
forced task as handled and will not re-run it repeatedly.
- Around line 1259-1343: The live reviewer output is appended directly to
this.state.currentOutput (and reviewStdout) in the onStdout handler so the UI
can’t show a divider between worker and reviewer streams; insert the review
divider into the live stream when the review stage begins by adding the divider
string to this.state.currentOutput (and reviewStdout) immediately before calling
reviewer.execute (or at the very start of the onStdout callback for the first
chunk), and also emit an 'agent:output' event with that divider (same shape as
used in onStdout) so the UI receives the split marker; update references in the
review-start block around reviewer.execute, the onStdout handler, and use the
existing iteration variable when emitting the divider.
In `@src/tui/components/IterationDetailView.tsx`:
- Around line 630-635: The current logic using
fullOutput.split(REVIEW_OUTPUT_DIVIDER) can lose content if the divider appears
inside reviewerOutput; change the split to locate the first divider via
fullOutput.indexOf(REVIEW_OUTPUT_DIVIDER) and use slice/substr to produce
workerOutput and reviewerOutput once only: if hasReviewOutput is true, set an
index const (e.g., idx = fullOutput.indexOf(REVIEW_OUTPUT_DIVIDER)), then
workerOutput = fullOutput.slice(0, idx) and reviewerOutput =
fullOutput.slice(idx + REVIEW_OUTPUT_DIVIDER.length); preserve the existing
variables (fullOutput, hasReviewOutput, REVIEW_OUTPUT_DIVIDER, workerOutput,
reviewerOutput) and fallback to [fullOutput, ''] when no divider is found.
In `@src/tui/components/RightPanel.tsx`:
- Around line 746-748: The comment above the isReviewEnabled declaration
contains temporal wording ("hasn't started yet"); update it to an evergreen
phrasing that does not reference time, e.g., change the comment to explain
intent generically such as "Ensure split sections are shown when a reviewer is
configured, even if no review output is present" and place it immediately above
the isReviewEnabled check (references: reviewerAgent and isReviewEnabled in
RightPanel.tsx).
- Around line 756-777: The current useMemo in RightPanel.tsx splits
iterationOutput with REVIEW_OUTPUT_DIVIDER which splits on every occurrence and
can truncate reviewer content; change the split logic so it only separates on
the first divider (e.g., use split with a limit of 2 or indexOf+substring) when
computing [worker, reviewer] so reviewerOutput retains any additional dividers;
update the branch that sets [worker, reviewer] (the code that currently uses
iterationOutput.split(REVIEW_OUTPUT_DIVIDER)) and keep subsequent logic using
stripAnsiCodes/isLiveStreaming and parseAgentOutput unchanged.
In `@src/tui/components/RunApp.tsx`:
- Line 1529: The reviewerConfigured boolean only checks
storedConfig?.review?.agent but must also ensure the review feature is enabled;
update the declaration of reviewerConfigured (used in RunApp.tsx) to require
both storedConfig?.review?.enabled === true and storedConfig?.review?.agent is
non-empty (trimmed). Locate the reviewerConfigured variable and change its
condition to mirror the UI checks (storedConfig?.review?.enabled &&
storedConfig?.review?.agent?.trim() !== ''), so Tab focus logic won't target the
reviewer pane when review is disabled.
In `@src/tui/components/SettingsView.tsx`:
- Around line 90-124: The getValue fallback chain is inconsistent with
validation; remove the implicit fallback to worker/default agent so the UI shows
only an explicit reviewer. In SettingsView.tsx update the reviewer field's
getValue to return config.review.enabled ? (config.review.agent ?? 'none') :
'none' (no use of config.agent or config.defaultAgent), keep setValue logic that
sets review.agent when a non-'none' value is chosen so review.agent remains
explicit, and leave requiresRestart as-is.
🧹 Nitpick comments (2)
src/tui/components/RightPanel.test.tsx (2)
12-36: Consider testing rendered output rather than source code patterns.This test reads the source file and asserts on string patterns, which is fragile:
- Formatting changes (whitespace, line breaks) can break the test even when behaviour is correct.
- Refactoring variable names or restructuring JSX would require updating the test.
A more robust approach would be to render the component and inspect the resulting element tree, or use snapshot testing for the rendered output. If source-level enforcement is truly needed, consider a linting rule instead.
38-47: Remove the no-op assertion or convert to a proper documentation comment.
expect(true).toBe(true)provides no testing value. If this is purely documentation, consider:
- Converting to a regular comment block outside of a test
- Adding it to a README or inline documentation in the component itself
- If keeping as a test, adding meaningful assertions about the documented behaviour
shawnsw
pushed a commit
to shawnsw/ralph-tui
that referenced
this pull request
Feb 4, 2026
Fixes all issues identified in CodeRabbit review of PR subsy#272. Config resolution: - Reuse primary agent config when review agent matches by name/plugin - Preserves command, timeout, agentOptions shorthands for reviewer Engine: - Insert REVIEW_OUTPUT_DIVIDER into live output stream - Mark forced task as processed when review blocks it - Prevents infinite loop in worker mode on review failure TUI: - Use indexOf/slice instead of split for output parsing - Prevents content loss if divider appears in reviewer output - Fix reviewerConfigured check to require review.enabled - Remove implicit reviewer fallback in SettingsView - Update temporal comment to evergreen phrasing Tests: - Remove no-op documentation test - Document why source-reading test is intentionally fragile - Move OpenTUI rendering rules to file header
|
Someone is attempting to deploy a commit to the plgeek Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/engine/index.ts (1)
1270-1473:⚠️ Potential issue | 🟡 MinorAvoid double/missing review divider in persisted stdout.
reviewStdoutis seeded with the divider and later overwritten; in the error path this can duplicate the divider, and if the reviewer emits no stdout the combined log can omit it. KeepreviewStdoutas raw reviewer output and add the divider once when combining.Proposed fix
- reviewStdout += REVIEW_OUTPUT_DIVIDER; this.emit({ type: 'agent:output', timestamp: new Date().toISOString(), stream: 'stdout', data: REVIEW_OUTPUT_DIVIDER, iteration, }); @@ - const combinedStdout = reviewStdout - ? `${agentResult.stdout}${REVIEW_OUTPUT_DIVIDER}${reviewStdout}` - : agentResult.stdout; + const combinedStdout = reviewEnabled + ? `${agentResult.stdout}${REVIEW_OUTPUT_DIVIDER}${reviewStdout}` + : agentResult.stdout;
🤖 Fix all issues with AI agents
In `@src/engine/index.ts`:
- Around line 369-380: Add a fail-fast validation for the reviewer model when
initializing the review agent: inside the block guarded by
this.config.review?.enabled (where reviewConfig, reviewInstance, reviewDetect
and this.reviewAgent are handled), run the same primary-model validation used
for the main agent (the same logic that validates this.config.agent.model)
against reviewConfig.model and throw a clear error if it is invalid before
calling agentRegistry.getInstance or reviewInstance.detect; in short, mirror the
primary validation path for config.review.model so a misconfigured review model
fails during initialization rather than at review time.
In `@src/tui/components/IterationDetailView.tsx`:
- Around line 963-1032: The reviewer pane is skipped when reviewerOutput is an
empty string causing focus to hide; change the rendering logic so when
hasReviewOutput is true you always render both the worker and reviewer sections
(use hasReviewOutput to gate the split view, not truthiness of
workerOutput/reviewerOutput) and inside each section use
renderOutputWithHighlighting(workerOutput || '(no output)') and
renderOutputWithHighlighting(reviewerOutput || '(no output)') (or a small
placeholder element) so empty strings show a visible placeholder; update the
conditions around the JSX for the worker and reviewer panes (references:
hasReviewOutput, workerOutput, reviewerOutput, outputFocus,
renderOutputWithHighlighting, SectionHeader, iteration.reviewAgent, workerAgent,
historicContext) to ensure panes are always present when review mode is enabled.
In `@src/tui/components/RightPanel.tsx`:
- Around line 746-751: The code currently hides the reviewer pane when review is
disabled (isReviewEnabled === false) even if iterationOutput contains
REVIEW_OUTPUT_DIVIDER, causing historical reviewer output to be dropped; change
the logic so the pane is shown if either review is configured or the current
output contains the divider. Specifically, replace or augment uses of
isReviewEnabled with a new/combined predicate (e.g., showReviewPane =
hasReviewOutput || (reviewerAgent !== undefined && reviewerAgent !== '')) and
use showReviewPane where the UI decides to keep the split and where reviewer
output is rendered so historical reviewer sections are preserved even when
reviewerAgent is empty. Ensure REVIEW_OUTPUT_DIVIDER, reviewerAgent,
iterationOutput, isReviewEnabled and hasReviewOutput references are updated
consistently.
🧹 Nitpick comments (1)
src/engine/index.ts (1)
177-178: Make the progress-summary comment evergreen.The comment uses temporal phrasing; please reword to avoid time-based language.
Proposed fix
- // Load recent progress for context (last 5 iterations) + // Load progress summary limited to 5 iterationsAs per coding guidelines, Avoid temporal context in comments; make them evergreen.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/tui/components/RunApp.tsx`:
- Around line 590-597: The comment in RunApp.tsx incorrectly states "Task list
is always visually highlighted"; update it to reflect that only the currently
selected task row remains highlighted regardless of focus while the LeftPanel's
border visual highlighting depends on focus. Specifically, edit the block around
the focusedPane state (useState<FocusedPane>('tasks')) to mention that LeftPanel
(LeftPanel component, isFocused prop) uses colors.accent.primary for the focused
border and colors.border.normal when not focused, and clarify that selectedIndex
controls per-row highlighting independent of pane focus.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/engine/index.ts`:
- Around line 1478-1484: The persisted combinedStdout drops
REVIEW_OUTPUT_DIVIDER when reviewEnabled is true but reviewStdout is empty,
preventing downstream viewers from splitting panes; update the logic around
combinedStdout (referencing combinedStdout, REVIEW_OUTPUT_DIVIDER,
reviewEnabled, reviewStdout, agentResult.stdout) to always append the divider
once reviewEnabled is true (even if reviewStdout is empty) so the persisted
output consistently contains the divider marker, while keeping actual
reviewStdout concatenation unchanged when present.
In `@src/tui/components/RunApp.tsx`:
- Around line 1527-1555: The Tab-focus cycling currently computes
reviewerConfigured only from storedConfig in RunApp.tsx, so setFocusedPane skips
the reviewer pane when the UI actually renders it due to historical output;
update the reviewer check to mirror RightPanel by treating the reviewer pane as
present if either storedConfig?.review?.enabled &&
storedConfig.review.agent?.trim() !== "" OR displayIterationOutput includes
REVIEW_OUTPUT_DIVIDER. Replace the reviewerConfigured variable (or its usage)
with this combined condition so the setFocusedPane cycling (the 'tab' case
logic) will include 'reviewer' whenever RightPanel would render the reviewer
pane; keep existing references to setFocusedPane, subagentPanelVisible,
detailsViewMode unchanged.
🧹 Nitpick comments (1)
src/tui/components/RightPanel.tsx (1)
16-19: Consider centralising the divider constant.This constant duplicates the engine's
REVIEW_OUTPUT_DIVIDER. If the value drifts between modules, output splitting will silently break. Consider exporting the constant from a shared location (e.g.,src/shared/constants.ts) or importing it from the engine module.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/engine/index.ts`:
- Around line 1392-1398: The final saved stdout currently only appends
REVIEW_OUTPUT_DIVIDER when reviewStdout.trim() is truthy, causing a mismatch
with the live stream that always inserts the divider when reviewEnabled is true;
update the construction of combinedStdout to include the REVIEW_OUTPUT_DIVIDER
whenever reviewEnabled is true (even if reviewStdout is empty) so logs match the
streamed output and tools that parse by divider (refer to combinedStdout,
REVIEW_OUTPUT_DIVIDER, reviewEnabled, reviewStdout); keep combinedStderr
behavior unchanged except ensure any conditional logic mirrors this same
reviewEnabled-based decision if you replicate divider logic elsewhere.
🧹 Nitpick comments (2)
src/config/index.ts (1)
438-462: Consider using a consistent naming pattern for dynamically created agent configs.When the registry has the plugin but no explicit config exists, the agent name is set to
review-${agentName}. This prefix may cause confusion when displayed in logs or the UI, since the agent name won't match the requested identifier. Consider using justagentNamefor consistency, or documenting the naming convention.💡 Suggested fix
if (registry.hasPlugin(agentName)) { return { - name: `review-${agentName}`, + name: agentName, plugin: agentName, options: {}, }; }src/templates/engine.ts (1)
590-645: Consider extracting shared rendering logic to reduce duplication.The
renderReviewPromptfunction shares significant code withrenderPrompt(context building, flat context creation, template compilation, and error handling). This duplication is manageable for now, but if more template types are added, consider extracting a commonrenderWithTemplate(loadResult, task, config, epic, extended)helper.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/tui/components/RunApp.tsx`:
- Line 2697: The reviewerAgent prop is currently set to undefined when review is
enabled but review.agent is unset; update the prop assignment in RunApp.tsx (and
the other occurrences where reviewerAgent is passed) to fall back to the primary
worker agent: use storedConfig?.review?.enabled ? (storedConfig?.review?.agent
?? storedConfig?.agent) : undefined (or equivalent) so that when review is
enabled and review.agent is absent the component receives storedConfig.agent
instead of undefined.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/tui/components/RunApp.tsx`:
- Around line 1599-1624: The Tab-cycling prevents focusing the reviewer pane
because reviewerConfigured requires a non-empty review.agent; change the
condition so reviewerConfigured is true whenever review is enabled (i.e., const
reviewerConfigured = !!storedConfig?.review?.enabled), so setFocusedPane(...)
will allow cycling to 'reviewer' even when review falls back to the worker
agent; update any references to storedConfig?.review?.agent trimming checks
accordingly in the detailsViewMode === 'prompt' branch so reviewer focus is
gated only by review enabled, not by agent string presence.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/tui/components/RunApp.tsx`:
- Around line 1555-1558: The Tab-key handler handleKeyboard uses
displayIterationOutput to detect REVIEW_OUTPUT_DIVIDER but
displayIterationOutput is not included in its useCallback dependency array,
causing a stale closure; update the useCallback that defines handleKeyboard to
include displayIterationOutput (and REVIEW_OUTPUT_DIVIDER if it's a
prop/variable) in its dependency array so reviewerPresent calculation
(reviewerEnabled/hasReviewerOutput) always uses the current
displayIterationOutput value and the Tab cycling logic behaves correctly.
🧹 Nitpick comments (1)
src/tui/components/RunApp.tsx (1)
1008-1050: Remote instances don't receive review prompt preview.When viewing a remote instance, only the worker prompt preview is fetched (lines 1010-1023). The review prompt preview is only generated for the local engine (lines 1037-1049). If review is enabled on a remote instance, users won't see the review prompt in prompt view mode.
Consider whether this is intentional or if
getRemotePromptPreviewshould also return a review prompt when applicable.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/tui/components/RunApp.tsx`:
- Around line 2348-2353: The reviewerAgentName computation can miss the resolved
worker agent (default/CLI override); update the useMemo that defines
reviewerAgentName to prefer the already-resolved agentName as the final fallback
instead of agentPlugin or storedConfig.agent alone: when
storedConfig.review.enabled is true, return storedConfig.review.agent?.trim() ||
agentName || storedConfig?.agent || agentPlugin (or equivalent ordering) so the
reviewer label matches the active worker agent; update the dependency array to
include agentName.
- Around line 1406-1428: hasReviewDividerInOutput currently derives the task to
check from displayedTasks[selectedIndex] and currentOutput which breaks when in
iterations or remote-output modes; update the memo to compute currentTaskId
based on viewMode and iterationSelectedIndex (use the same selection logic
RightPanel uses), and when isViewingRemote include remote/iteration
agentResult.stdout and the remote historicalOutputCache entries in the checks;
continue to fallback to currentOutput and displayedTasks for the default view
and still check REVIEW_OUTPUT_DIVIDER against currentOutput,
taskIteration.agentResult.stdout, and historicalData.output so Tab logic aligns
with the rendered RightPanel.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tui/components/RunApp.tsx (1)
1571-1607:⚠️ Potential issue | 🟠 MajorGuard Shift+A to prevent double-invoking both accept and accept-all actions.
When Shift+A is pressed,
key.shiftis true andkey.nameis'a', socase 'a'executes first, callingonConflictAccept. The subsequentif (key.shift && key.name === 'a')block then runs, callingonConflictAcceptAll. Both handlers fire in the same keystroke, triggering unintended double actions.Use
key.sequence === 'A'(uppercase, matching the pattern used elsewhere in this file for Shift+C, Shift+O, and Shift+T) to check for Shift+A before the switch statement and return early.Proposed fix
if (showConflictPanel) { + if (key.sequence === 'A') { + if (onConflictAcceptAll) { + onConflictAcceptAll(); + } + setShowConflictPanel(false); + return; + } switch (key.name) { case 'escape': // Abort conflict resolution and rollback if (onConflictAbort) { onConflictAbort().catch(() => {}); } setShowConflictPanel(false); break; case 'j': case 'down': setConflictSelectedIndex((prev) => Math.min(prev + 1, parallelConflicts.length - 1)); break; case 'k': case 'up': setConflictSelectedIndex((prev) => Math.max(prev - 1, 0)); break; case 'a': // Accept AI resolution for the selected file if (onConflictAccept && parallelConflicts[conflictSelectedIndex]) { onConflictAccept(parallelConflicts[conflictSelectedIndex].filePath); } break; case 'r': // Reject - abort conflict resolution for this merge if (onConflictAbort) { onConflictAbort().catch(() => {}); } setShowConflictPanel(false); break; } - // Handle shift+A for Accept All (key.shift is true when shift is held) - if (key.shift && key.name === 'a') { - if (onConflictAcceptAll) { - onConflictAcceptAll(); - } - setShowConflictPanel(false); - } return; }
🤖 Fix all issues with AI agents
In `@src/tui/components/RightPanel.tsx`:
- Around line 381-385: The map over displayLabels in RightPanel.tsx uses
key={label}, which can collide after sanitisation; change the key to a stable
unique value such as the index or a combined identifier (e.g., `${label}-${i}`)
in the render expression where displayLabels.map((label, i) => ...) so each
<span> gets a non-duplicating key; update the key usage in that mapping (the
span with key={label}) to use the new unique key strategy.
- Around line 908-910: The code currently uses a falsy check "if
(!iterationOutput)" inside the useMemo that collapses empty strings to
undefined; change this to an explicit null/undefined check (e.g., "if
(iterationOutput == null)" or "if (iterationOutput === undefined ||
iterationOutput === null)") so that empty string outputs are preserved for
workerOutput/reviewerOutput; update the block around the useMemo that computes
workerOutput and reviewerOutput (referencing iterationOutput, workerOutput,
reviewerOutput) to return undefined only for null/undefined iterationOutput and
allow empty strings to pass through.
- Around line 281-287: The metadata sanitiser function sanitizeMetadataValue is
over-applying control-character regexes causing Biome's
noControlCharactersInRegex errors; remove the redundant .replace(/\x1b./g, '')
and .replace(/[\x00-\x1F\x7F]/g, '') calls since stripAnsiCodes(value) together
with .replace(/\p{C}/gu, '') already strips control characters—update the
sanitizeMetadataValue implementation in RightPanel.tsx and the identical
function in TaskDetailView.tsx to only use
stripAnsiCodes(value).replace(/\p{C}/gu, '').trim() (preserving the initial
falsy check and return type).
In `@src/tui/components/TaskDetailView.tsx`:
- Around line 259-263: The current map over displayLabels in TaskDetailView.tsx
uses key={label}, which can collide after label sanitisation; update the key to
ensure uniqueness (e.g., combine the sanitized label with the map index or
another stable identifier) in the map callback that renders the outer span for
each label so keys are unique even when labels collapse.
- Around line 171-177: The sanitizeMetadataValue function contains redundant
control-character regex replacements flagged by Biome; remove the
.replace(/\x1b./g, '') and .replace(/[\x00-\x1F\x7F]/g, '') calls since
stripAnsiCodes(value) and .replace(/\p{C}/gu, '') already handle ANSI and
Unicode control characters — update sanitizeMetadataValue in TaskDetailView.tsx
accordingly and apply the same change to the analogous sanitizer in
RightPanel.tsx to keep behavior consistent.
In `@src/tui/theme.ts`:
- Around line 416-423: The dist detection using pathSegments.includes('dist')
can produce false positives; change it to check only the current directory name
(e.g., use path.basename(currentDir) === 'dist' or compare
pathSegments[pathSegments.length - 1] === 'dist') so isInDist accurately
reflects running from the dist folder; update the logic around currentDir,
pathSegments, sep, isInDist and the subsequent return join(currentDir, 'assets',
'themes') accordingly.
🧹 Nitpick comments (3)
src/config/schema.test.ts (1)
487-539: Consider adding a nested integer check for parallel.maxWorkers.This would mirror the standalone ParallelConfigSchema integer test within StoredConfigSchema.
💡 Optional test addition
test('validates parallel.maxWorkers bounds', () => { expect(() => StoredConfigSchema.parse({ parallel: { maxWorkers: 0 } })).toThrow(); expect(() => StoredConfigSchema.parse({ parallel: { maxWorkers: 33 } })).toThrow(); }); + + test('validates parallel.maxWorkers is integer', () => { + expect(() => StoredConfigSchema.parse({ parallel: { maxWorkers: 2.5 } })).toThrow(); + });src/commands/run.tsx (1)
1990-2014: Consider whetheronConflictAcceptshould provide more feedback.The
onConflictAcceptcallback only triggers a rerender when the resolution is successful. Whenresolution?.successis falsy (either not found or failed), nothing happens - the user gets no feedback.This may be intentional given the comment "primarily for user feedback", but consider whether a failed/missing resolution should surface an indication to the user.
src/commands/run.test.ts (1)
522-646: Consider whether keyboard handler tests add sufficient value.These tests simulate the callback logic patterns from
RunApp.tsxrather than testing actual keyboard handling. While they document the expected contract, they:
- Don't import or reference actual handlers from
RunApp.tsx- Manually re-implement the logic being tested
- Could become stale if
RunApp.tsxchangesIf the goal is to ensure callback contracts are honoured, consider either:
- Adding comments noting these are contract documentation tests
- Or moving this logic into shared utility functions that both the component and tests can use
As-is, they serve as documentation of expected behaviour, which has value.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/commands/run.tsx`:
- Around line 739-748: areAllConflictsResolved currently uses
resolutions.find(...) which returns the first matching resolution and can be
wrong when duplicate resolution entries exist for the same file; change the
lookup to use the latest resolution per file (e.g., pick the last matching
ConflictResolutionResult in the resolutions array for a given conflict.filePath,
or use Array.prototype.findLast if available) so that the function checks the
most recent resolution.success for each FileConflict rather than the first one.
🧹 Nitpick comments (2)
src/tui/components/TaskDetailView.tsx (1)
171-192: Consider extracting metadata sanitisation into a shared helper.
TaskDetailView and RightPanel’s metadata view now share near‑identical sanitisation/timestamp logic; a small util would reduce drift.src/tui/components/RunApp.tsx (1)
66-68: Consider centralising REVIEW_OUTPUT_DIVIDER.
The divider string now exists here and in RightPanel; a shared constant would prevent drift.
Comment on lines
+739
to
+748
| export function areAllConflictsResolved( | ||
| conflicts: FileConflict[], | ||
| resolutions: ConflictResolutionResult[] | ||
| ): boolean { | ||
| if (conflicts.length === 0) return true; | ||
| if (resolutions.length < conflicts.length) return false; | ||
| return conflicts.every((conflict) => { | ||
| const resolution = resolutions.find((r) => r.filePath === conflict.filePath); | ||
| return resolution?.success === true; | ||
| }); |
Contributor
There was a problem hiding this comment.
Handle duplicate resolution entries when checking completion.
areAllConflictsResolved uses find, so if a file has multiple entries (e.g., a placeholder added before AI resolution), the first match can keep the session blocked even after a successful resolution is appended.
🔧 Suggested fix (prefer latest resolution per file)
export function areAllConflictsResolved(
conflicts: FileConflict[],
resolutions: ConflictResolutionResult[]
): boolean {
if (conflicts.length === 0) return true;
if (resolutions.length < conflicts.length) return false;
- return conflicts.every((conflict) => {
- const resolution = resolutions.find((r) => r.filePath === conflict.filePath);
- return resolution?.success === true;
- });
+ const latestByPath = new Map<string, ConflictResolutionResult>();
+ for (const resolution of resolutions) {
+ latestByPath.set(resolution.filePath, resolution);
+ }
+ return conflicts.every((conflict) => {
+ return latestByPath.get(conflict.filePath)?.success === true;
+ });
}📝 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
| export function areAllConflictsResolved( | |
| conflicts: FileConflict[], | |
| resolutions: ConflictResolutionResult[] | |
| ): boolean { | |
| if (conflicts.length === 0) return true; | |
| if (resolutions.length < conflicts.length) return false; | |
| return conflicts.every((conflict) => { | |
| const resolution = resolutions.find((r) => r.filePath === conflict.filePath); | |
| return resolution?.success === true; | |
| }); | |
| export function areAllConflictsResolved( | |
| conflicts: FileConflict[], | |
| resolutions: ConflictResolutionResult[] | |
| ): boolean { | |
| if (conflicts.length === 0) return true; | |
| if (resolutions.length < conflicts.length) return false; | |
| const latestByPath = new Map<string, ConflictResolutionResult>(); | |
| for (const resolution of resolutions) { | |
| latestByPath.set(resolution.filePath, resolution); | |
| } | |
| return conflicts.every((conflict) => { | |
| return latestByPath.get(conflict.filePath)?.success === true; | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@src/commands/run.tsx` around lines 739 - 748, areAllConflictsResolved
currently uses resolutions.find(...) which returns the first matching resolution
and can be wrong when duplicate resolution entries exist for the same file;
change the lookup to use the latest resolution per file (e.g., pick the last
matching ConflictResolutionResult in the resolutions array for a given
conflict.filePath, or use Array.prototype.findLast if available) so that the
function checks the most recent resolution.success for each FileConflict rather
than the first one.
shawnsw
pushed a commit
to shawnsw/ralph-tui
that referenced
this pull request
Feb 5, 2026
Fixes all issues identified in CodeRabbit review of PR subsy#272. Config resolution: - Reuse primary agent config when review agent matches by name/plugin - Preserves command, timeout, agentOptions shorthands for reviewer Engine: - Insert REVIEW_OUTPUT_DIVIDER into live output stream - Mark forced task as processed when review blocks it - Prevents infinite loop in worker mode on review failure TUI: - Use indexOf/slice instead of split for output parsing - Prevents content loss if divider appears in reviewer output - Fix reviewerConfigured check to require review.enabled - Remove implicit reviewer fallback in SettingsView - Update temporal comment to evergreen phrasing Tests: - Remove no-op documentation test - Document why source-reading test is intentionally fragile - Move OpenTUI rendering rules to file header
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/engine/index.ts`:
- Around line 1242-1250: The UI fails to detect the reviewer split because
REVIEW_OUTPUT_DIVIDER is only emitted via this.emit in the engine and not added
to this.state.currentOutput; update the emit block around this.emit(...) to also
append REVIEW_OUTPUT_DIVIDER into this.state.currentOutput so RunApp.tsx's
currentOutput.includes(REVIEW_OUTPUT_DIVIDER) becomes true during
streaming—ensure you avoid duplication by checking state.currentOutput (e.g.,
last entry or includes) before pushing; touch the same area where this.emit is
called and reference REVIEW_OUTPUT_DIVIDER, this.emit, and
this.state.currentOutput when making the change.
🧹 Nitpick comments (1)
src/templates/builtin.ts (1)
426-434: Consider wrapping PRD content in<details>for consistency with other templates.Other templates (e.g.,
BEADS_TEMPLATE,JSON_TEMPLATE) wrap the full PRD document in a collapsible<details>element. This review template displays it inline. If the PRD is lengthy, this could dominate the prompt.🔧 Optional: Add collapsible wrapper for PRD content
{{`#if` prdContent}} ## PRD: {{prdName}} {{`#if` prdDescription}} {{prdDescription}} {{/if}} +<details> +<summary>Full PRD Document (click to expand)</summary> + +{{prdContent}} + +</details> + ### Progress {{prdCompletedCount}}/{{prdTotalCount}} tasks complete -{{/if}} +{{else}} +### Progress +{{prdCompletedCount}}/{{prdTotalCount}} tasks complete +{{/if}}
Comment on lines
+1242
to
+1250
| // Emit divider to live output stream so UI can split worker and reviewer in real-time | ||
| // Note: Don't add to this.state.currentOutput to avoid duplication in final combined log | ||
| this.emit({ | ||
| type: 'agent:output', | ||
| timestamp: new Date().toISOString(), | ||
| stream: 'stdout', | ||
| data: REVIEW_OUTPUT_DIVIDER, | ||
| iteration, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Identify currentOutput consumers to verify whether live split relies on state
rg -n "currentOutput" src -g'*.ts*' -C3Repository: subsy/ralph-tui
Length of output: 11105
🏁 Script executed:
#!/bin/bash
# Check the context of the divider check in RunApp.tsx
sed -n '1435,1450p' src/tui/components/RunApp.tsxRepository: subsy/ralph-tui
Length of output: 629
🏁 Script executed:
#!/bin/bash
# Find REVIEW_OUTPUT_DIVIDER definition
rg -n "REVIEW_OUTPUT_DIVIDER" src -g'*.ts*' -C2Repository: subsy/ralph-tui
Length of output: 5868
Divider excluded from live output breaks UI split detection during streaming.
The divider is emitted only via agent:output events and intentionally not added to state.currentOutput (to avoid duplication in saved logs). However, the UI checks currentOutput.includes(REVIEW_OUTPUT_DIVIDER) at RunApp.tsx:1442 to detect reviewer output. For live iterations, this check always fails, preventing the UI from splitting worker and reviewer panels until after the iteration completes and loads from saved state.
Either add the divider to currentOutput during streaming (if duplication can be handled), or derive the split from agent:output events rather than state inspection.
🤖 Prompt for AI Agents
In `@src/engine/index.ts` around lines 1242 - 1250, The UI fails to detect the
reviewer split because REVIEW_OUTPUT_DIVIDER is only emitted via this.emit in
the engine and not added to this.state.currentOutput; update the emit block
around this.emit(...) to also append REVIEW_OUTPUT_DIVIDER into
this.state.currentOutput so RunApp.tsx's
currentOutput.includes(REVIEW_OUTPUT_DIVIDER) becomes true during
streaming—ensure you avoid duplication by checking state.currentOutput (e.g.,
last entry or includes) before pushing; touch the same area where this.emit is
called and reference REVIEW_OUTPUT_DIVIDER, this.emit, and
this.state.currentOutput when making the change.
Allow codex agent to run in non-git directories by adding skipGitRepoCheck config option (default: true). Fixes issue where codex would fail with "Not inside a trusted directory" error when running in directories without git repos. - Add skipGitRepoCheck option to CodexAgentPlugin - Pass --skip-git-repo-check flag to codex exec when enabled - Add configuration question in setup wizard - Add comprehensive test coverage
Split worker and reviewer outputs into separate scrollable sections with Tab key navigation, update header to show both agents, and reorganize iteration detail layout. Changes: - Split combined output using REVIEW_OUTPUT_DIVIDER - Add Tab key navigation between worker/reviewer sections - Show both agents in header when review enabled (worker: X | reviewer: Y) - Move iteration number to timing info line - Reorganize iteration detail view layout - Add visual focus indicators for active output section - Pass workerAgent prop to IterationDetailView - Pass reviewerAgent from StoredConfig to Header Improves clarity when review stage is enabled by clearly separating worker execution from reviewer feedback.
This reverts commit 2237068f9f23076acb1ce4e49e106b43c0fdc1ed. The skipGitRepoCheck option bypassed codex's safety check, which is not the right approach. We'll implement git repository checking at the ralph-tui layer instead with a friendly prompt to initialize git.
Add interactive prompt when running in non-git directory with options: - Initialize git repository (recommended) - Exit and initialize manually - Continue without git (not recommended) Implementation: - Add isGitRepository() to check for .git directory - Add initGitRepository() to run git init - Add promptGitInit() for user interaction - Check runs before engine start in interactive mode only - Skip check in headless/CI mode to avoid blocking automation This properly addresses the issue instead of bypassing codex's --skip-git-repo-check safety mechanism.
…ader - Add REVIEW_OUTPUT_DIVIDER constant to split worker and reviewer output - Separate output into two independent scrollable sections when review is enabled - Each section shows clear title: "Worker: <agent>" and "Reviewer: <agent>" - Remove redundant agent name display from task header, show only model info - Add reviewerAgent prop to RightPanel, TaskDetails, and TaskOutputView components - Improve layout: task title remains prominent, timing info preserved - Fix output duplication by properly parsing and splitting combined output
…gation - Show split worker/reviewer sections when review is enabled (not just when divider exists) - This ensures sections appear immediately during live execution - Add Tab key navigation to switch focus between worker and reviewer outputs - Focused section has highlighted border (accent color vs normal) - Show helpful status messages: "Waiting for reviewer..." during execution - Add focus indicator hint: "[Worker focused - Tab to switch]" - Reset focus to worker when iteration changes Fixes the issue where split sections only appeared in historical view but not during live execution.
Fixes multiple UX issues with split worker/reviewer output: - Task list now loses focus when Tab switches to output sections - Mouse wheel scrolling works in focused output sections via scrollbox focused prop - Headers (Worker/Reviewer/Tasks) remain visible when unfocused using custom text styling - Completed tasks readable when selected (improved color contrast) - Iteration number restored in worker header - ANSI escape sequences properly stripped (including corrupted U+FFFD variants) Prevents nested <text> components by using <span> elements for multi-colored headers, fixing "TextNodeRenderable only accepts strings" runtime error.
Fixes all issues identified in CodeRabbit review of PR subsy#272. Config resolution: - Reuse primary agent config when review agent matches by name/plugin - Preserves command, timeout, agentOptions shorthands for reviewer Engine: - Insert REVIEW_OUTPUT_DIVIDER into live output stream - Mark forced task as processed when review blocks it - Prevents infinite loop in worker mode on review failure TUI: - Use indexOf/slice instead of split for output parsing - Prevents content loss if divider appears in reviewer output - Fix reviewerConfigured check to require review.enabled - Remove implicit reviewer fallback in SettingsView - Update temporal comment to evergreen phrasing Tests: - Remove no-op documentation test - Document why source-reading test is intentionally fragile - Move OpenTUI rendering rules to file header
- Add model validation for review agent during initialization - Fix review divider handling to avoid duplication in error paths - Update temporal comment phrasing to evergreen style - Always render both panes when divider present (show placeholders for empty output) - Preserve split layout when divider exists even if review disabled - Clarify focus state comment to distinguish panel borders from row highlighting - Change Details view to only make Description section focusable/scrollable - Update type definitions for content focus mode
…tern Add template resolution hierarchy for review prompts matching worker templates: - CLI flag: --review-prompt overrides everything - Project: .ralph-tui/templates/review.hbs - Global: ~/.config/ralph-tui/templates/review.hbs - Built-in: REVIEW_TEMPLATE constant Changes: - Add REVIEW_TEMPLATE constant in templates/builtin.ts - Add loadReviewTemplate() function with unified hierarchy - Add renderReviewPrompt() for review prompt rendering - Update buildReviewPrompt() to use new template system - Remove review.prompt_template config field (new feature, no users yet) - Add reviewPromptPath to RalphConfig (CLI flag only) Benefits: - Consistent API between worker and review templates - Users can configure review templates globally or per-project - Clean implementation without unnecessary config fields
- Add review prompt preview in prompt view alongside worker prompt - Implement generateReviewPromptPreview() method in engine for review template rendering - Fix 2x duplication in reviewer output (removed duplicate divider insertion) - Add responsive layouts for all views (Details, Output, Prompt): * Wide screens (≥160 cols): side-by-side layout for better readability * Narrow screens (<160 cols): stacked layout for efficient space use - Improve prompt view Tab cycling: tasks → worker → reviewer → tasks - Simplify template source labels (global:/path → global, cli flag paths → cli) - Make reviewer prompt heading more visible with accent color Enhances the TUI experience by showing both worker and reviewer prompts before execution, and optimizing layouts for different terminal sizes. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Always include review divider in saved output when review enabled Ensures consistency between live stream and final logs Makes it explicit that review ran even with empty output - Align Tab focus cycling with actual reviewer pane rendering Check for both configured reviewer AND divider in output Allows focusing reviewer pane with historical output
added 5 commits
February 5, 2026 12:38
…t set When review is enabled but review.agent is unset, the engine falls back to using the primary agent for review. The UI now reflects this by: - Computing reviewerAgentName with fallback to primary agent - Displaying the correct agent name in Header and RightPanel components - Matching the engine's behavior for consistency
Previously, Tab cycling checked for both review.enabled AND review.agent being set. Since the engine falls back to the primary agent when review.agent is unset, the Tab cycling should only check if review is enabled, not if review.agent is specifically configured. Changes: - Output view: Check review.enabled instead of review.agent presence - Prompt view: Check review.enabled instead of review.agent presence - Added clarifying comments about the fallback behavior
Fixed stale closure in Tab cycling logic by: - Creating hasReviewDividerInOutput useMemo to check for review divider - Using displayedTasks/selectedIndex to avoid dependency on selectedTask - Adding hasReviewDividerInOutput to handleKeyboard dependency array - Using pre-computed value instead of inline displayIterationOutput check This ensures Tab cycling always uses current output state when determining if reviewer pane should be included in the focus cycle.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/engine/index.ts`:
- Around line 294-301: The code currently picks reviewModel as
this.config.review.model ?? this.config.model and always calls
reviewInstance.validateModel(reviewModel), which incorrectly validates the
worker model against the reviewer when review.model is unset and the reviewer
plugin differs; change the logic so you only validate when a review model is
explicitly specified OR when the reviewer plugin is the same as the worker
plugin: compute the model to validate by using this.config.review.model if
present, otherwise only fall back to this.config.model when
this.config.review.plugin === this.config.plugin, and then call
reviewInstance.validateModel(reviewModel) only when that computed reviewModel is
defined.
In `@src/tui/components/RunApp.tsx`:
- Around line 2867-2871: IterationDetailView is receiving
workerAgent={agentPlugin} which can be undefined and causes the worker label to
be blank; instead pass the resolved agent name prop (agentName) so the UI always
shows the active agent. Replace the workerAgent={agentPlugin} prop on the
IterationDetailView invocation with agentName={agentName} (or the existing
resolved agent identifier variable you use elsewhere, e.g. resolvedAgentName)
while keeping the other props (sandboxConfig, resolvedSandboxMode,
historicContext/iterationDetailHistoricContext) unchanged.
- Around line 1012-1016: The reviewer pane collapses because
setReviewPromptPreview is being set to undefined during loading; instead of
clearing reviewPromptPreview (and reviewTemplateSource) to undefined in the
loading branch, set them to a non-empty placeholder (e.g., "Loading review
preview...") so PromptPreviewView stays rendered and the split layout remains
stable while generating; update the loading logic that calls
setReviewPromptPreview and setReviewTemplateSource (alongside setPromptPreview
and setTemplateSource) to use a placeholder string when review mode is enabled
so the reviewer pane stays visible.
Comment on lines
+294
to
+301
| // Validate review model if specified | ||
| const reviewModel = this.config.review.model ?? this.config.model; | ||
| if (reviewModel) { | ||
| const modelError = reviewInstance.validateModel(reviewModel); | ||
| if (modelError) { | ||
| throw new Error(`Review model validation failed: ${modelError}`); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid validating a worker-only model against a different reviewer plugin.
If the reviewer plugin differs and review.model is unset, the current logic validates the worker model against the reviewer, which can throw even though the review run would use the reviewer’s default model. Align validation with the execution-time model selection.
💡 Suggested fix
- const reviewModel = this.config.review.model ?? this.config.model;
+ const reviewModel =
+ this.config.review.model ??
+ (reviewInstance.meta.id === this.agent?.meta.id ? this.config.model : undefined);🤖 Prompt for AI Agents
In `@src/engine/index.ts` around lines 294 - 301, The code currently picks
reviewModel as this.config.review.model ?? this.config.model and always calls
reviewInstance.validateModel(reviewModel), which incorrectly validates the
worker model against the reviewer when review.model is unset and the reviewer
plugin differs; change the logic so you only validate when a review model is
explicitly specified OR when the reviewer plugin is the same as the worker
plugin: compute the model to validate by using this.config.review.model if
present, otherwise only fall back to this.config.model when
this.config.review.plugin === this.config.plugin, and then call
reviewInstance.validateModel(reviewModel) only when that computed reviewModel is
defined.
Comment on lines
1012
to
1016
| setPromptPreview('Generating prompt preview...'); | ||
| setTemplateSource(undefined); | ||
| setReviewPromptPreview(undefined); | ||
| setReviewTemplateSource(undefined); | ||
|
|
Contributor
There was a problem hiding this comment.
Keep the reviewer pane visible while the review prompt preview is loading.
Clearing reviewPromptPreview to undefined while review is enabled makes PromptPreviewView collapse to a single pane, even though Tab can focus the reviewer. Consider setting a non-empty placeholder while loading so the split layout stays stable.
💡 Suggested adjustment
- setReviewPromptPreview(undefined);
- setReviewTemplateSource(undefined);
+ setReviewPromptPreview(storedConfig?.review?.enabled ? 'Loading review prompt...' : undefined);
+ setReviewTemplateSource(undefined);🤖 Prompt for AI Agents
In `@src/tui/components/RunApp.tsx` around lines 1012 - 1016, The reviewer pane
collapses because setReviewPromptPreview is being set to undefined during
loading; instead of clearing reviewPromptPreview (and reviewTemplateSource) to
undefined in the loading branch, set them to a non-empty placeholder (e.g.,
"Loading review preview...") so PromptPreviewView stays rendered and the split
layout remains stable while generating; update the loading logic that calls
setReviewPromptPreview and setReviewTemplateSource (alongside setPromptPreview
and setTemplateSource) to use a placeholder string when review mode is enabled
so the reviewer pane stays visible.
Comment on lines
2867
to
2871
| sandboxConfig={sandboxConfig} | ||
| resolvedSandboxMode={resolvedSandboxMode} | ||
| historicContext={iterationDetailHistoricContext} | ||
| workerAgent={agentPlugin} | ||
| /> |
Contributor
There was a problem hiding this comment.
Use the resolved agent name for IterationDetailView.
agentPlugin can be undefined when the active agent is resolved via defaults or CLI overrides, which leaves the worker label blank. Passing agentName (resolved) avoids that.
💡 Suggested adjustment
- workerAgent={agentPlugin}
+ workerAgent={agentName}📝 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
| sandboxConfig={sandboxConfig} | |
| resolvedSandboxMode={resolvedSandboxMode} | |
| historicContext={iterationDetailHistoricContext} | |
| workerAgent={agentPlugin} | |
| /> | |
| sandboxConfig={sandboxConfig} | |
| resolvedSandboxMode={resolvedSandboxMode} | |
| historicContext={iterationDetailHistoricContext} | |
| workerAgent={agentName} | |
| /> |
🤖 Prompt for AI Agents
In `@src/tui/components/RunApp.tsx` around lines 2867 - 2871, IterationDetailView
is receiving workerAgent={agentPlugin} which can be undefined and causes the
worker label to be blank; instead pass the resolved agent name prop (agentName)
so the UI always shows the active agent. Replace the workerAgent={agentPlugin}
prop on the IterationDetailView invocation with agentName={agentName} (or the
existing resolved agent identifier variable you use elsewhere, e.g.
resolvedAgentName) while keeping the other props (sandboxConfig,
resolvedSandboxMode, historicContext/iterationDetailHistoricContext) unchanged.
Author
|
@subsy This PR adds the review process, updated UI to split worker/reviewer output with Tab navigation. I've addressed multiple rounds of CodeRabbit feedback and resolved all merge conflicts. While CodeRabbit continues to find edge cases, the core functionality is complete and working. Could you review from a feature perspective? |
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.
Summary
Adds split worker/reviewer output view with keyboard navigation and improved focus management in the TUI.
Changes
Core Features
UX Improvements
Bug Fixes
Infrastructure
Test Plan
Breaking Changes
None - all changes are additive or internal improvements.
Additional Context
This PR builds on the existing reviewer functionality and enhances it with a dedicated split-view UI. The keyboard navigation and focus management make it easier to review agent output during long-running tasks.
All commits rebased on latest upstream (dda82e5).
Summary by CodeRabbit
New Features
Bug Fixes
Tests