[Fix] Approval controls disappear when user scrolls through chat - #1448
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Review processThis PR was opened by an automated account. A human maintainer must verify the change intent, provenance, and validation before merging.
Current step: The required review sequence passed. Remaining merge requirements apply. |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (6)Require regression coverage at the lowest valid harness with behavior-focused assertions, including relevant negative, error, false/unset, and boundary cases. Check cleanup and deterministic async behavior and prefer shared typed test helpe...⚙️ CodeRabbit configuration file Files:
Check strict typing and exhaustive behavior across normal, boundary, error, cancellation, retry, and compatibility paths. Verify promises and errors are handled, existing helpers are reused, and new code introduces no `any`, unjustified dou...⚙️ CodeRabbit configuration file Files:
Check React state and effect dependencies, cleanup, accessibility, i18n, and light/dark theme behavior. New markup should use Tailwind; add VS Code CSS variables to `src/index.css` before Tailwind use. Use Vitest for behavior and Playwright...⚙️ CodeRabbit configuration file Files:
Act as an adversarial second-opinion reviewer. Verify PR claims against implementation, contracts, and tests. Trace changed inputs through normal, boundary, error, cancellation, retry, and default paths and their consumers. Seek plausible c...⚙️ CodeRabbit configuration file Files:
Add focused tests for UI binding and save behavior, persistence or normalization, and the value returned by `getStateToPostToWebview()`, including true and false/unset cases when defaults could hide omissions.📄 CodeRabbit inference engine (AGENTS.md) Files:
Fix lint violations in new TypeScript code instead of suppressing them.📄 CodeRabbit inference engine (AGENTS.md) Files:
🔇 Additional comments (2)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesChat action-row behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change keeps existing approval controls visible while users browse earlier chat output and adds regression coverage for the affected interaction. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
Full details: Description checkExplanation The description explains the problem, implementation, impact, linked issue, and regression coverage. It does not reproduce every template section, such as the checklist and a detailed test procedure, but the core information is complete. Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. Full details: Regression EvidenceExplanation The focused JSDOM regression test covers the main Resolution Add a Playwright component visual test and committed baseline for the ChatView action row. Cover the scrolled state with the compact scroll-to-bottom control beside an approval control, checkpoint suppression, and a long localized label in a narrow layout. Add a focused Vitest test that preserves Full details: Trust And Persistence InvariantsExplanation PASS: No changed path meets the custom failure conditions. The PR changes only
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
webview-ui/src/components/chat/ChatView.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. webview-ui/src/components/chat/__tests__/ChatView.scroll-debug-repro.spec.tsxESLint skipped: the matched ESLint configuration already failed (missing-dependency). 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 |
|
Coverage follow-up shipped in |
What changed
Why this change was made
Scrolling previously replaced the entire action row with navigation controls. A child task waiting for completion approval therefore appeared stuck because the only control that could return to its parent was no longer rendered.
Closes #1447.
Impact
Users can approve subtask completion and other blocking actions without first changing their chat scroll position. The regression was developed test-first and the full repository suite passes. A post-fix extension screenshot is not included because this task's approved proof browser exposes only the documentation site, not the VS Code Extension Host surface.
Related PRs