Skip to content

fix(web): use selected provider for pre-session health banner#25

Merged
zortos293 merged 1 commit intozortos293:mainfrom
viraj-s15:fix/pre-session-provider-health-banner
Mar 13, 2026
Merged

fix(web): use selected provider for pre-session health banner#25
zortos293 merged 1 commit intozortos293:mainfrom
viraj-s15:fix/pre-session-provider-health-banner

Conversation

@viraj-s15
Copy link
Copy Markdown
Contributor

@viraj-s15 viraj-s15 commented Mar 13, 2026 •
edited by coderabbitai bot
Loading

What Changed

  • Added a small UI-only helper in apps/web/src/components/ChatView.logic.ts to resolve which provider’s health banner should be shown.
  • Updated apps/web/src/components/ChatView.tsx so that when a thread has no active session yet (pre-session), the banner follows the currently selected draft provider instead of defaulting to Codex.
  • Added regression tests in apps/web/src/components/ChatView.logic.test.ts to cover:
    • session provider takes priority when present
    • selected draft provider is used before session start

Why

When creating a new thread from the sidebar +, the thread starts in a pre-session state; previously, banner provider resolution fell back to Codex, which could show:

Codex CLI (codex) is not installed or not on PATH.

for Copilot-first workflows where Codex is intentionally not installed.

This change keeps behavior minimal and focused: it only adjusts banner-provider resolution in the web UI and does not modify provider runtime/server protocol flow.

UI Changes

Before (Codex warning shown even in Copilot workflow pre-session):

Before: Codex warning in pre-session draft thread

After:

  • In pre-session draft threads, provider health banner now follows the selected draft provider.
  • Codex warning is no longer shown unless Codex is actually the active/intended provider.

With Copilot:
260313_20h40m44s_screenshot

With Codex:
260313_20h42m45s_screenshot

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for UI changes (before included, after can be added)
  • I included a video for animation/interaction changes (N/A — no animation/interaction behavior changed)

Refs #24

Summary by CodeRabbit

  • Tests

    • Added comprehensive unit tests for provider health banner selection logic, covering multiple scenarios including active session handling and fallback provider selection.

  • Refactor

    • Refactored provider resolution logic by extracting the selection mechanism into a dedicated helper function to enhance code maintainability, testability, and overall separation of concerns across components.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 13, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9b14902d-1d4d-4ac8-bd6d-df0cd1f2fff9

📥 Commits

Reviewing files that changed from the base of the PR and between dfb8e3b and b55d6f1.

📒 Files selected for processing (3)
  • apps/web/src/components/ChatView.logic.test.ts
  • apps/web/src/components/ChatView.logic.ts
  • apps/web/src/components/ChatView.tsx
📝 Walkthrough

Walkthrough

This pull request introduces a new helper function resolveProviderHealthBannerProvider that encapsulates provider selection logic in a dedicated module. The function returns the session provider when available, otherwise falls back to the selected provider. The logic is extracted from inline null-coalescing in ChatView.tsx and refactored into a separate utility function with comprehensive unit test coverage via a new test file.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing provider health banner logic to use the selected provider in pre-session state.
Description check ✅ Passed The description comprehensively covers what changed, why it was needed, and includes before/after UI screenshots demonstrating the fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

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

Tip

You can customize the high-level summary generated by CodeRabbit.

Configure the reviews.high_level_summary_instructions setting to provide custom instructions for generating the high-level summary.

@zortos293 zortos293 merged commit ae4310d into zortos293:main Mar 13, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

size:M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@viraj-s15 @zortos293