Repository navigation
fix(ui): keep SpinnerModeGlyph visible inside status parens - #2047
Conversation
📝 WalkthroughWalkthrough
ChangesSpinner layout behavior
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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/Spinner/SpinnerAnimationRow.test.tsx`:
- Around line 74-218: Replace the partial assertions in the SpinnerAnimationRow
regression tests with exact assertions against the complete ANSI-stripped
rendered row, covering requesting, leader thinking, narrow-width glyph/thinking
and bare-thinking fallbacks, teammate status, and reduced-motion cases. Preserve
each test’s intended width and mode setup, and assert exact separators,
parentheses, status text, and absence of extra output rather than using
toContain or partial regex matching.
🪄 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: 3699a781-e781-4667-b132-d0f1bd9f803b
📒 Files selected for processing (2)
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.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/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.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/Spinner/SpinnerAnimationRow.test.tsx
🔇 Additional comments (1)
src/components/Spinner/SpinnerAnimationRow.test.tsx (1)
31-218: 📐 Maintainability & Code QualityValidation results are unavailable.
No focused test or TypeScript validation result is included, so this draft is not merge-ready from CodeRabbit’s side. Run and report
bun test ./src/components/Spinner/SpinnerAnimationRow.test.tsxandbun run typecheck; also runbun run typecheck:type-testsif configured. As per coding guidelines, “Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.” As per path instructions, “Run the narrowest useful validation commands and report the exact ones used.”Sources: Coding guidelines, Path instructions
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/Spinner/SpinnerAnimationRow.tsx (1)
170-353: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftFollow-up (not this PR): extract the gating cascade into a pure function.
This is now ~180 lines of five interdependent passes over mutable
letflags, all inside a component that re-runs every 50ms. Every case is currently pinned only by end-to-end render assertions at specific column counts, which makes the next width tweak expensive to validate. AGENTS.md rightly says keep this fix focused, so not a change request — but a follow-up extractingresolveStatusLayout(inputs) → {showThinking, showTimer, showTokens, showGlyph, thinkingText}would let the width matrix be table-tested directly instead of through Ink.Want me to open an issue for it?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Spinner/SpinnerAnimationRow.tsx` around lines 170 - 353, The reviewer identifies a follow-up refactor rather than a change required in this PR. Keep the progressive gating cascade in the current component unchanged, and do not extract a pure layout resolver or add related tests as part of this fix.Source: Path instructions
src/components/Spinner/SpinnerAnimationRow.test.tsx (1)
179-195: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWhitespace-significant expectations deserve a note.
(${figures.arrowUp} )and${figures.arrowDown} ·encode the<Box width={2}>glyph padding as literal trailing/double spaces. Correct, but invisible in review and one stray formatter pass from a confusing failure. A one-line comment (or aGLYPH = (f) => \${f} `` helper) would make the padding intentional on its face.Non-blocking nit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Spinner/SpinnerAnimationRow.test.tsx` around lines 179 - 195, Clarify the intentional whitespace in the expectations within the SpinnerAnimationRow tests, especially the padded glyph expressions using figures.arrowUp and figures.arrowDown. Add a concise inline comment or shared helper documenting the glyph padding, while preserving the existing whitespace-sensitive output assertions.
🤖 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/Spinner/SpinnerAnimationRow.test.tsx`:
- Line 321: Update the regex assertions in SpinnerAnimationRow tests, including
the assertion near line 321 and the one near line 479, to interpolate the
existing PROD_MESSAGE constant instead of hardcoding “Thinking…”. Preserve the
current regex structure and assertion behavior.
In `@src/components/Spinner/SpinnerAnimationRow.tsx`:
- Around line 248-269: Clarify the tie-break in the `preferTokensOverTimerOnly`
branch comment so it explicitly states that tokens take precedence over thinking
when both preferences are true. Add a focused test covering the layout width
where `preferTokensOverTimerOnly` and `preferThinkingOverTimerOnly` are both
true, asserting tokens remain visible and thinking is hidden.
- Around line 284-296: The suppressModeGlyphForBareThinking assignment in the
token-only fallback branch is redundant and misleading. Remove this assignment,
relying on reserveModeGlyph = false to prevent canShowModeGlyph from enabling
the glyph, or rename the flag to a state-neutral symbol such as
glyphDroppedForContent and update both suppression paths consistently.
- Around line 221-234: Document the intentional boundary split in the bare pass
near bareShowThinking and bareShowTimer: explain that thinking and timer require
`>` to retain an extra safety column, while token-related checks use `>=`
because they may consume the exact physical budget. Keep the existing conditions
unchanged.
---
Outside diff comments:
In `@src/components/Spinner/SpinnerAnimationRow.test.tsx`:
- Around line 179-195: Clarify the intentional whitespace in the expectations
within the SpinnerAnimationRow tests, especially the padded glyph expressions
using figures.arrowUp and figures.arrowDown. Add a concise inline comment or
shared helper documenting the glyph padding, while preserving the existing
whitespace-sensitive output assertions.
In `@src/components/Spinner/SpinnerAnimationRow.tsx`:
- Around line 170-353: The reviewer identifies a follow-up refactor rather than
a change required in this PR. Keep the progressive gating cascade in the current
component unchanged, and do not extract a pure layout resolver or add related
tests as part of this fix.
🪄 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: 89749055-c58e-4dc0-8108-b3f8c2f38ce1
📒 Files selected for processing (2)
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.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/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.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/Spinner/SpinnerAnimationRow.test.tsx
🔇 Additional comments (10)
src/components/Spinner/SpinnerAnimationRow.tsx (5)
174-201: LGTM!
302-338: 📐 Maintainability & Code Quality | 💤 Low valueGlyph-only
(↓ )at narrow widths spends 4 columns to say nothing.When nothing else fits (e.g. columns=21 for
Thinking…), the fallback chain leaves(↓ )— arrow, padding space, parens. That is exactly the "empty glyph-only row" the comment at lines 298-301 says it wants to avoid, just with an arrow in it. The numeric-duration case is correctly excluded viaallowGlyphOnlyStatus; consider whether active-thinking deserves the same treatment whenhasVisibleStatusContentis false, or confirm the arrow alone is the intended cue.Non-blocking, but the test at lines 235-252 locks this in, so decide deliberately.
339-353: LGTM!
362-397: LGTM!
411-440: 📐 Maintainability & Code QualityNo action: this is checked-in source. The React Compiler cache pattern is already used throughout
src/components/**, so the switch reorder won’t be overwritten by a build step.> Likely an incorrect or invalid review comment.src/components/Spinner/SpinnerAnimationRow.test.tsx (5)
13-23: LGTM!
72-141: LGTM!
199-266: LGTM!
337-459: LGTM!
304-321: 🩺 Stability & AvailabilityNo cleanup issue here.
renderToStringalready waits forRenderOnceAndExitto exit, so thereducedMotion: falsecases don't leave the animation loop running.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Spinner/SpinnerAnimationRow.tsx (1)
172-398: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider extracting the width-gating cascade into a pure, unit-testable function.
This block now runs 4+ sequential passes (initial gating, bare-glyph recompute, suffix-drop, duration/thinking recovery) over mutable
lets, and it's already produced two rounds of subtle bugs (the timer-recovery gap above, plus the several tie-break/documentation issues fixed in prior commits). Pulling the width math into a standalone function returning{ showThinking, showTimer, showTokens, showSuffix, reserveModeGlyph, thinkingText }would let each gating rule be unit-tested directly againstcolumns/suffixWidth/etc. without going through full component rendering, which would likely have caught the timer gap.Non-blocking for this draft PR, but worth considering before this logic grows further.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Spinner/SpinnerAnimationRow.tsx` around lines 172 - 398, Consider extracting the mutable width-gating cascade from SpinnerAnimationRow into a standalone pure helper that accepts the relevant layout inputs and returns showThinking, showTimer, showTokens, showSuffix, reserveModeGlyph, and thinkingText (plus any required derived values). Move the initial gating, bare-glyph recomputation, suffix-drop, and recovery rules into that helper while preserving their current ordering and tie-break behavior, so the rules can be unit-tested independently of component rendering.
🤖 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/Spinner/SpinnerAnimationRow.tsx`:
- Around line 288-316: Update the suffix-drop recovery in SpinnerAnimationRow so
showTimer is restored when the suffix is removed and the elapsed timer fits the
freed physicalBareBudget, including the teammate/no-thinking/no-token-content
scenario. Handle timer and token recovery symmetrically, preserving the existing
width accounting and allowing both to be shown when they fit together; add
regression tests in SpinnerAnimationRow.test.tsx covering the narrow teammate
suffix-drop case and combined timer/token recovery.
---
Outside diff comments:
In `@src/components/Spinner/SpinnerAnimationRow.tsx`:
- Around line 172-398: Consider extracting the mutable width-gating cascade from
SpinnerAnimationRow into a standalone pure helper that accepts the relevant
layout inputs and returns showThinking, showTimer, showTokens, showSuffix,
reserveModeGlyph, and thinkingText (plus any required derived values). Move the
initial gating, bare-glyph recomputation, suffix-drop, and recovery rules into
that helper while preserving their current ordering and tie-break behavior, so
the rules can be unit-tested independently of component rendering.
🪄 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: df0bcf2d-ae09-4754-9f85-f8bd08d68012
📒 Files selected for processing (2)
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.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/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.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/Spinner/SpinnerAnimationRow.test.tsx
🔇 Additional comments (3)
src/components/Spinner/SpinnerAnimationRow.tsx (1)
407-450: LGTM!src/components/Spinner/SpinnerAnimationRow.test.tsx (2)
140-178: LGTM! The suffix-drop token recovery test loops across the boundary columns, and the glyph-drop-to-keep-suffix test correctly asserts the exact single-row output.
197-213: LGTM! Traced this against the implementation (hasRunningTeammates→reserveModeGlyph=false→bareThinkingOnlypath) and the expected nested(thinking)output matches.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/Spinner/SpinnerAnimationRow.test.tsx`:
- Around line 197-216: Strengthen the assertions in the “prefers streaming
tokens over a bare-fitting suffix” test by verifying that visibleRows(output)
contains exactly one row before checking its contents. Keep the existing
token-presence and suffix-absence assertions unchanged.
In `@src/components/Spinner/SpinnerAnimationRow.tsx`:
- Around line 339-387: Update the recovery logic in the !showSuffix block so a
row with showThinking true and both showTimer and showTokens false re-evaluates
tokens after the suffix is dropped. Restore showTokens when tokensFitBareAlone
and the remaining budget accommodates the visible thinking content plus the
required separator, updating the corresponding used-width bookkeeping and
mode-glyph reservation consistently with the existing recovery branches.
🪄 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: 562055bb-80da-48a4-8859-dbba5229462f
📒 Files selected for processing (2)
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.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/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.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/Spinner/SpinnerAnimationRow.test.tsx
🔇 Additional comments (7)
src/components/Spinner/SpinnerAnimationRow.tsx (4)
245-296: LGTM!
297-334: LGTM!
412-468: LGTM!Also applies to: 469-517
542-565: LGTM!Also applies to: 572-572
src/components/Spinner/SpinnerAnimationRow.test.tsx (3)
143-175: LGTM!Also applies to: 218-271
273-338: LGTM!Also applies to: 340-361, 363-425
527-556: LGTM!Also applies to: 763-793
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/Spinner/SpinnerAnimationRow.tsx`:
- Around line 327-334: Update the suffix-fit predicate in SpinnerAnimationRow’s
layout logic to include the rendered timer width and full thinking width when
showTimer is enabled and thinkingStatus is “thinking” with an effortSuffix. Base
the calculation on the actual suffix-plus-timer-plus-thinking/tokens layout so
showSuffix is disabled when that combination overflows, and add an exact
boundary test covering this scenario.
- Around line 380-386: In the token-recovery branch of SpinnerAnimationRow, when
tokens are restored, also validate whether the current withGlyphSpace budget can
accommodate the mode glyph; clear reserveModeGlyph when it cannot so the arrow
is not re-added. Add narrow-width coverage for recovered thinking plus tokens
both with and without the glyph.
🪄 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: f158fe70-e70c-47be-8f4b-5e2e04c73084
📒 Files selected for processing (1)
src/components/Spinner/SpinnerAnimationRow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/components/Spinner/SpinnerAnimationRow.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/Spinner/SpinnerAnimationRow.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/components/Spinner/SpinnerAnimationRow.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/Spinner/SpinnerAnimationRow.tsx
🔇 Additional comments (1)
src/components/Spinner/SpinnerAnimationRow.tsx (1)
603-621: LGTM!Also applies to: 630-631
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/Spinner/SpinnerAnimationRow.tsx`:
- Around line 335-378: Collapse the redundant suffix-fit cascade in
SpinnerAnimationRow by relying on the earlier allVisibleFitWithSuffix check
instead of separately handling !tokensWithSuffixFit and
!tokensWithSuffixAndThinkingFit. Remove any now-unused fit calculations, and
simplify timer visibility handling around timerWantedVisible without changing
the intended showTimer behavior.
🪄 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: be2ce0a8-1f3f-491c-86a4-a9a045807e3e
📒 Files selected for processing (2)
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.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/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.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/Spinner/SpinnerAnimationRow.test.tsx
🔇 Additional comments (4)
src/components/Spinner/SpinnerAnimationRow.tsx (2)
454-463: LGTM! Correctly reconcilesreserveModeGlyphagainst the recovered-tokens total, addressing the earlier "glyph re-added after overflow" concern.
401-476: 📐 Maintainability & Code QualityThese width trackers are still active
usedAfterThinking/usedAfterTimerstill feed the later recovery paths, timer re-entry, and glyph gating, so this isn’t dead bookkeeping.> Likely an incorrect or invalid review comment.src/components/Spinner/SpinnerAnimationRow.test.tsx (2)
213-213: LGTM! Closes the previously-flagged gap by asserting a single row before checking content.
915-1015: LGTM! Good monotonicity coverage across column bands with frozen refs for determinism, consistent with the AGENTS.md guidance on deterministic spinner tests. Manually traced the width math for the columns=36 "recovers thinking plus tokens..." case (995-1015) and the expected(6s · 1.0k tokens)output (timer+tokens preferred over thinking+tokens when the suffix hard-overflows) checks out against the production gating logic.
Always render the ↑/↓ mode glyph for leader spins so early requesting and thinking-only phases are not blank, and place it as the first status part inside the parentheses next to other activity cues. Closes Twigpine#2033
Restore a second-chance width gate for leader thinking-only status under the new inside-parens glyph layout, and suppress the mode glyph when the row cannot fit minimal status chrome.
When leader thinking-only cannot fit glyph+thinking chrome, fall back to bare (thinking) instead of empty mode-glyph status. Tighten glyph residual budget to account for GlimmerMessage trailing space.
Bare leader thinking-only residual must reserve the GlimmerMessage trailing space so equality-width terminals do not overflow by one column.
Apply the bareThinkingOnly nested (thinking) wrap in both shimmer and dimColor branches so teammate thinking-only status keeps parentheses when reduced motion disables the shimmer arm.
Fix TS1355 from invalid null as const in baseProps and replace partial toContain/regex checks with full ANSI-stripped row equality for the glyph placement regressions CodeRabbit requested.
When reserving the SpinnerModeGlyph would drop tokens/timer from the status row, drop the glyph instead. Keep thinking full-chrome recovery, default unknown modes to down-arrow, and tighten exact-row tests for typecheck plus CodeRabbit feedback.
Skip glyph-only status when numeric thinkingStatus cannot fit on narrow terminals, and refuse glyph-free recovery that would swap visible tokens for a timer-only layout. Co-authored-by: Cursor <cursoragent@cursor.com>
Exclude numeric post-thinking duration from glyph-only status (including requesting), prefer streaming tokens over duration when both cannot fit, and recover timer+token rows at the col-30 boundary.
Prefer tokens over timer/duration on mid-narrow rows, recover exact-fit token columns, keep full effort text when glyph chrome fits, and prefer active thinking over timer-only status. Tests now use production Thinking… message width and frozen-clock exact row assertions.
Document token-over-thinking tie-break and > vs >= bare-pass split, drop redundant suppressModeGlyph assignments on token-only fallbacks, and make exact-row tests use PROD_MESSAGE plus an explicit padded mode-glyph helper.
Recover mid-narrow status when stop-hook/tool suffixes overflow bare chrome, prefer live tokens over duration after the drop, and cover long production verb column bands plus suffix regressions.
Re-gate tokens when thinkingStatus is null after dropping a crowding suffix, restore teammate nested thinking on the same path, and drop the mode glyph before truncating a suffix that still fits under bare parens.
Restore timer symmetrically after suffix drop, drop crowding suffixes when preferTokens would overflow, keep already-visible thinking when tokens unlock, and prefer tokens over a bare-fitting suffix that cannot share the row.
Budget timer co-restore against all visible parts, re-gate tokens onto timer-only rows after tokens-over-suffix, prefer thinking over a crowding bare-fit suffix, and restore the mode glyph only after a suffix-keep cascade.
Prefer tokens over thinking when they cannot share bare chrome, tighten thinking-over-suffix exact-fit to avoid a one-column suffix cliff, and restore the mode glyph whenever recovered leader content fits.
Budget timer and rendered thinking width in suffix-fit predicates so widening does not drop the elapsed timer or streaming tokens. Co-restore teammate tokens when thinking crowds timer-only rows, clear the mode glyph when thinking+token recovery cannot fit glyph chrome, and budget full effort text before keeping a stop-hook suffix.
Rely on the combined all-visible suffix budget check instead of duplicate tokens-only paths. Keeps timer+thinking and no-token thinking fallbacks unchanged.
a82bc76 to
c452760
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/Spinner/SpinnerAnimationRow.tsx (1)
335-338: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
timerWantedVisiblestill re-deriveswantsTimer.The parenthesized clause is character-for-character the definition of
wantsTimer(Line 186), so this collapses toshowTimer || wantsTimer, and sinceshowTimeris only ever set under awantsTimerguard, towantsTimer. Same redundancy flagged previously; still present.♻️ Proposed simplification
- const timerWantedVisible = - showTimer || - (wantsTimer && - (verbose || hasRunningTeammates || effectiveElapsedMs > SHOW_TIMER_AFTER_MS)); + const timerWantedVisible = showTimer || wantsTimer;Non-blocking.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Spinner/SpinnerAnimationRow.tsx` around lines 335 - 338, Update the timerWantedVisible calculation in SpinnerAnimationRow to reuse the existing wantsTimer value directly instead of repeating its verbose, teammate, and elapsed-time conditions; preserve the resulting showTimer-or-wantsTimer visibility behavior.
🤖 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/Spinner/SpinnerAnimationRow.test.tsx`:
- Around line 402-405: Update the test case around visibleRows(output) to assert
that rows has exactly one element before checking its contents, matching the
sibling width-loop tests and ensuring wrapped layouts fail.
---
Duplicate comments:
In `@src/components/Spinner/SpinnerAnimationRow.tsx`:
- Around line 335-338: Update the timerWantedVisible calculation in
SpinnerAnimationRow to reuse the existing wantsTimer value directly instead of
repeating its verbose, teammate, and elapsed-time conditions; preserve the
resulting showTimer-or-wantsTimer visibility behavior.
🪄 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: 13bb6428-7acf-4f1f-b75c-dbfeb686cc2c
📒 Files selected for processing (2)
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: smoke-and-tests (22)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/components/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.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/Spinner/SpinnerAnimationRow.tsxsrc/components/Spinner/SpinnerAnimationRow.test.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/Spinner/SpinnerAnimationRow.test.tsx
🔇 Additional comments (11)
src/components/Spinner/SpinnerAnimationRow.tsx (6)
172-216: LGTM!
220-296: LGTM!
376-455: LGTM!
456-606: LGTM!
616-650: LGTM!
659-693: LGTM!src/components/Spinner/SpinnerAnimationRow.test.tsx (5)
2-63: LGTM!
66-175: LGTM!
177-294: LGTM!
463-714: LGTM!
716-1056: LGTM!
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Spinner/SpinnerAnimationRow.tsx (1)
327-371: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore full effort text after suffix removal.
The suffix-fit pass budgets
fullThinkingWidth, but post-suffix recovery reuses the existingthinkingWidthValue. If the suffix previously forcedthinking with high effortdown to barethinking, removing that suffix never restores the full text—even when the newly available width fits it alongside tokens or the timer. Recompute the thinking text/width after suffix removal and add an exact regression case for this transition.As per coding guidelines, “Add or update tests when a TypeScript or TSX change affects behavior.” As per path instructions, “Because this PR changes user-facing terminal UI behavior, add/update tests to lock deterministic rendering.”
Also applies to: 377-451, 547-561
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Spinner/SpinnerAnimationRow.tsx` around lines 327 - 371, Update the suffix-removal recovery logic in SpinnerAnimationRow so removing the suffix recomputes the thinking text and width from the full effort value, rather than reusing thinkingWidthValue; preserve the full effort text when the newly available space fits it with tokens or the timer. Apply the same correction to the related recovery paths around the referenced logic, and add a deterministic regression test covering high-effort thinking being reduced to bare “thinking” and restored after suffix removal.Sources: Coding guidelines, Path instructions
🤖 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.
Outside diff comments:
In `@src/components/Spinner/SpinnerAnimationRow.tsx`:
- Around line 327-371: Update the suffix-removal recovery logic in
SpinnerAnimationRow so removing the suffix recomputes the thinking text and
width from the full effort value, rather than reusing thinkingWidthValue;
preserve the full effort text when the newly available space fits it with tokens
or the timer. Apply the same correction to the related recovery paths around the
referenced logic, and add a deterministic regression test covering high-effort
thinking being reduced to bare “thinking” and restored after suffix removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 468611af-7e79-457e-a1be-dd53a2534b76
📒 Files selected for processing (2)
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, includingbun run typecheckand, when applicable,bun run typecheck:type-tests.
Files:
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI components under
src/components/.
Files:
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.tsx
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns indocs/integrations/overview.mdand the focused guides underdocs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Usebun installto install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.
Files:
src/components/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.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/Spinner/SpinnerAnimationRow.test.tsxsrc/components/Spinner/SpinnerAnimationRow.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/Spinner/SpinnerAnimationRow.test.tsx
🔇 Additional comments (2)
src/components/Spinner/SpinnerAnimationRow.tsx (1)
172-216: LGTM!Also applies to: 220-296, 297-326, 456-546, 562-603, 613-632, 643-643, 662-680, 690-690
src/components/Spinner/SpinnerAnimationRow.test.tsx (1)
2-63: LGTM!Also applies to: 66-68, 70-79, 81-99, 101-119, 121-175, 177-217, 219-253, 255-294, 296-361, 363-460, 462-514, 515-676, 679-715, 716-809, 811-914, 915-942, 944-964, 966-993, 995-1014, 1017-1056
…#2047) * fix(ui): keep SpinnerModeGlyph visible inside status parens Always render the ↑/↓ mode glyph for leader spins so early requesting and thinking-only phases are not blank, and place it as the first status part inside the parentheses next to other activity cues. Closes Twigpine#2033 * fix(ui): preserve narrow-terminal thinking with mode glyph Restore a second-chance width gate for leader thinking-only status under the new inside-parens glyph layout, and suppress the mode glyph when the row cannot fit minimal status chrome. * fix(ui): prefer bare thinking over glyph-only on narrow rows When leader thinking-only cannot fit glyph+thinking chrome, fall back to bare (thinking) instead of empty mode-glyph status. Tighten glyph residual budget to account for GlimmerMessage trailing space. * fix(ui): budget glimmer space in bare thinking fallback Bare leader thinking-only residual must reserve the GlimmerMessage trailing space so equality-width terminals do not overflow by one column. * fix(ui): nest teammate bare thinking under reduced motion Apply the bareThinkingOnly nested (thinking) wrap in both shimmer and dimColor branches so teammate thinking-only status keeps parentheses when reduced motion disables the shimmer arm. * test(ui): assert exact SpinnerAnimationRow status rows Fix TS1355 from invalid null as const in baseProps and replace partial toContain/regex checks with full ANSI-stripped row equality for the glyph placement regressions CodeRabbit requested. * fix(ui): prefer status content over empty mode-glyph chrome When reserving the SpinnerModeGlyph would drop tokens/timer from the status row, drop the glyph instead. Keep thinking full-chrome recovery, default unknown modes to down-arrow, and tighten exact-row tests for typecheck plus CodeRabbit feedback. * fix(ui): preserve spinner tokens when glyph crowds status * fix(ui): suppress empty glyph chrome and preserve token recovery Skip glyph-only status when numeric thinkingStatus cannot fit on narrow terminals, and refuse glyph-free recovery that would swap visible tokens for a timer-only layout. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(ui): tighten glyph recovery for duration and timer bands Exclude numeric post-thinking duration from glyph-only status (including requesting), prefer streaming tokens over duration when both cannot fit, and recover timer+token rows at the col-30 boundary. * fix(ui): harden SpinnerAnimationRow glyph recovery priorities Prefer tokens over timer/duration on mid-narrow rows, recover exact-fit token columns, keep full effort text when glyph chrome fits, and prefer active thinking over timer-only status. Tests now use production Thinking… message width and frozen-clock exact row assertions. * fix(ui): address CodeRabbit SpinnerAnimationRow recovery nits Document token-over-thinking tie-break and > vs >= bare-pass split, drop redundant suppressModeGlyph assignments on token-only fallbacks, and make exact-row tests use PROD_MESSAGE plus an explicit padded mode-glyph helper. * fix(ui): drop overflowing spinner suffix for tokens/thinking Recover mid-narrow status when stop-hook/tool suffixes overflow bare chrome, prefer live tokens over duration after the drop, and cover long production verb column bands plus suffix regressions. * fix(ui): recover status after SpinnerAnimationRow suffix overflow Re-gate tokens when thinkingStatus is null after dropping a crowding suffix, restore teammate nested thinking on the same path, and drop the mode glyph before truncating a suffix that still fits under bare parens. * fix(ui): complete SpinnerAnimationRow suffix and glyph recovery Restore timer symmetrically after suffix drop, drop crowding suffixes when preferTokens would overflow, keep already-visible thinking when tokens unlock, and prefer tokens over a bare-fitting suffix that cannot share the row. * fix(ui): harden SpinnerAnimationRow recovery against wrap cliffs Budget timer co-restore against all visible parts, re-gate tokens onto timer-only rows after tokens-over-suffix, prefer thinking over a crowding bare-fit suffix, and restore the mode glyph only after a suffix-keep cascade. * fix(ui): close SpinnerAnimationRow mid-narrow recovery cliffs Prefer tokens over thinking when they cannot share bare chrome, tighten thinking-over-suffix exact-fit to avoid a one-column suffix cliff, and restore the mode glyph whenever recovered leader content fits. * test(ui): cover SpinnerAnimationRow cliff and glyph-restore cases * fix(ui): restore tokens beside thinking after suffix recovery * fix(ui): close SpinnerAnimationRow suffix recovery cliffs Budget timer and rendered thinking width in suffix-fit predicates so widening does not drop the elapsed timer or streaming tokens. Co-restore teammate tokens when thinking crowds timer-only rows, clear the mode glyph when thinking+token recovery cannot fit glyph chrome, and budget full effort text before keeping a stop-hook suffix. * refactor(ui): collapse redundant SpinnerAnimationRow suffix-fit branches Rely on the combined all-visible suffix budget check instead of duplicate tokens-only paths. Keeps timer+thinking and no-token thinking fallbacks unchanged. * fix(ui): re-gate recovered spinner glyph * test(ui): tighten spinner layout coverage --------- Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Summary
Fixes SpinnerModeGlyph visibility and placement bugs from #2033.
Acceptance
partsinside( … ), not floating left of the opening paren.(thinking)rather than empty glyph-only chrome.(thinking)under reduced motion as well as shimmer.Root cause
SpinnerModeGlyphwas unshifted only when!thinkingOnly && parts.length > 0, so early requesting and thinking-only phases dropped the arrow. Later width-recovery passes also left timer/token/thinking re-gating incomplete after suffix overflow.Implementation
(↓ · thinking)then bare(thinking).thinkingDisplayfor teammate bare nesting in both motion arms.Stretch item (token-trend semantics) left out of this minimum viable fix.
Test plan
bun test --feature=UNATTENDED_RETRY --max-concurrency=1 ./src/components/Spinner/SpinnerAnimationRow.test.tsx(45 pass)./src/utils/context.test.ts(pass)Closes #2033
Final reviewed SHA: 84af711
Summary by CodeRabbit