Repository navigation
fix(effort): keep effort indicator visible in prompt footer - #1919
Conversation
📝 WalkthroughWalkthroughAdds a shared ChangesEffort footer fallback
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant PromptInput
participant Notifications
participant AppState
participant NotificationContent
PromptInput->>Notifications: render notification area
Notifications->>AppState: read effortValue and notification state
Notifications->>Notifications: evaluate IDE and KAIROS fallback visibility
Notifications->>NotificationContent: pass notification and fallback inputs
NotificationContent->>NotificationContent: render active notification or effort fallback
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/PromptInput/Notifications.effort.test.tsx`:
- Around line 171-178: Add a `briefFeatureTest` in
`Notifications.effort.test.tsx` to cover the case where `isBriefOnly` is true
but `viewingAgentTaskId` is set, since `briefOwnsGap` should not suppress effort
in that path. Use `renderNotifications` with `effortValue: 'medium'`,
`isBriefOnly: true`, and a non-empty `viewingAgentTaskId`, then assert that the
output does contain the effort text (for example the `medium · /effort` label)
to verify the override behavior in `Notifications`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b80d8e20-4cc1-4219-82ea-20c427961d0f
📒 Files selected for processing (4)
src/components/IdeStatusIndicator.tsxsrc/components/PromptInput/Notifications.effort.test.tsxsrc/components/PromptInput/Notifications.tsxsrc/components/PromptInput/PromptInput.tsx
💤 Files with no reviewable changes (1)
- src/components/PromptInput/PromptInput.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
Files:
src/components/IdeStatusIndicator.tsxsrc/components/PromptInput/Notifications.effort.test.tsxsrc/components/PromptInput/Notifications.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/IdeStatusIndicator.tsxsrc/components/PromptInput/Notifications.effort.test.tsxsrc/components/PromptInput/Notifications.tsx
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.src/integrations/- provider and model integration metadata.src/entrypoints/- CLI, MCP, SDK, and generated public types.src/tasks/- local, remote, workflow, and monitor tas...
Files:
src/components/IdeStatusIndicator.tsxsrc/components/PromptInput/Notifications.effort.test.tsxsrc/components/PromptInput/Notifications.tsx
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/components/IdeStatusIndicator.tsxsrc/components/PromptInput/Notifications.effort.test.tsxsrc/components/PromptInput/Notifications.tsx
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/components/PromptInput/Notifications.effort.test.tsx
🪛 React Doctor (0.5.8)
src/components/PromptInput/Notifications.tsx
[warning] 286-286: This component misses React Compiler's automatic memoization & re-renders more than it should: Hooks must always be called in a consistent order, and may not be called conditionally. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning). Rewrite the flagged code so the compiler can optimize it.
(hooks)
🔇 Additional comments (3)
src/components/IdeStatusIndicator.tsx (1)
13-26: LGTM!src/components/PromptInput/Notifications.tsx (1)
284-314: Effort fallback logic is correct.Precedence chain is sound: transient notification (
notificationNode) → effort fallback (effortFallbackNode) → nothing. Empty-text notifications correctly leavenotificationNodeasnull(empty string is falsy), so the effort fallback fills in.getEffortNotificationTextreturningundefinedfor unsupported models safely produces anullfallback node. ThebriefOwnsGap/shouldShowIdeSelectiongating matches the PR objectives.The React Doctor warning at line 286 is a false positive —
feature()is a compile-time constant with an existingbiome-ignorepattern, so hook call order is stable at runtime. Same pattern asVOICE_MODEandisBriefOnlyabove.Source: Linters/SAST tools
src/components/PromptInput/Notifications.effort.test.tsx (1)
48-96: 🎯 Functional CorrectnessNo action needed here.
useMainLoopModel()already prefersmainLoopModelForSession, thenmainLoopModel, then the default, so this test is wired to the intended model.> Likely an incorrect or invalid review comment.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found two test coverage issues that need to be addressed before this is ready.
Findings
-
[P2] Add a mounted update regression for the
/effortchange path
src/components/PromptInput/Notifications.effort.test.tsx:105
Issue #1831 is not only that the footer eventually disappears; it also says the displayed effort does not update immediately after changing/effort. The current "uses current app state" test renders two separateAppStateProviderinitial states (highOutputandlowOutput), so it would not catch a regression whereNotificationsonly reads the initialeffortValueand fails to repaint when the existing store is updated by/effort. Please add a mounted-state regression that starts with one effort value, updateseffortValuethrough the same AppState store, and asserts the rendered footer changes to the new effort without remounting the component. -
[P2] Complete CodeRabbit's brief-mode coverage request
src/components/PromptInput/Notifications.effort.test.tsx:171
The patch now depends on thebriefOwnsGap = isBriefOnly && !viewingAgentTaskIdbranch, which means effort is suppressed in brief mode only while the main prompt owns the footer gap; when a teammate task is being viewed, the effort fallback should render again. The current tests only cover the suppression case with noviewingAgentTaskId, so a future "simplification" could drop the teammate-view exception and still leave this new test file green. CodeRabbit has an unresolved requested change for exactly this branch, and the repo's contributor policy requires automated review findings to be addressed before maintainer review proceeds. Please add the requestedbriefFeatureTestcase withisBriefOnly: true, a non-emptyviewingAgentTaskId, and an assertion thatmedium · /effortrenders.
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the previously discussed paths and do not see any remaining actionable issues from my side.
@kevincodex1 LGTM
b761084
Hanxcellent
changed the title
…#1919) * fix: keep effort indicator visible * fix: preserve footer status for effort fallback * fix: simplify effort footer selection logic * fix: harden effort footer fallback * test: cover brief effort footer suppression * fix: ignore empty footer notifications * test: cover live effort footer updates * fix: handle empty JSX footer notifications
…#1919) * fix: keep effort indicator visible * fix: preserve footer status for effort fallback * fix: simplify effort footer selection logic * fix: harden effort footer fallback * test: cover brief effort footer suppression * fix: ignore empty footer notifications * test: cover live effort footer updates * fix: handle empty JSX footer notifications
Summary
Fixes #1831
Impact
Testing
bun run buildviabun run check/bun run smokebun run smokeviabun run checkbun run check(exits 1 locally; compared withorigin/main, both have the same 75 unique failing tests and no branch-only failures)bun test src/components/PromptInput/Notifications.effort.test.tsxbun test --feature=KAIROS_BRIEF src/components/PromptInput/Notifications.effort.test.tsxCLAUDE_CODE_EFFORT_LEVEL=high bun test --feature=KAIROS_BRIEF src/components/PromptInput/Notifications.effort.test.tsxNotes
bun run checkcurrently has existing failures onorigin/main; this branch does not add any new failing test casesSummary by CodeRabbit
Bug Fixes
Refactor
Tests