Skip to content

Conversation

@Agastya18
Copy link
Contributor

@Agastya18 Agastya18 commented Oct 29, 2025

Fix inconsistent capitalization in LiteLLM model alias variable names to prevent naming collisions and improve code clarity.

Fix: #717

Before

  • Local state: litellmModelAlias (lowercase 'llm')
  • Hook function: setLiteLLMModelAlias (uppercase 'LLM')
  • Naming collision and inconsistent capitalization

After

  • Local state: liteLLMModelAliasInput (uppercase 'LLM' + 'Input' suffix)
  • Hook function: setLiteLLMModelAlias (uppercase 'LLM')
  • No collision, consistent naming convention

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Oct 29, 2025
@sentry
Copy link

sentry bot commented Oct 29, 2025

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: client/src/components/SettingsTab.tsx

Function Unhandled Issue
SettingsTab ReferenceError: getLiteLLMBaseUrl is not defined /
Event Count: 2 Affected Users: 1
SettingsTab Error: Rendered more hooks than during the previous render. ...
Event Count: 1 Affected Users: 1

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 29, 2025

Walkthrough

The change renames an internal state variable in SettingsTab from litellmModelAlias to liteLLMModelAliasInput. This refactoring updates all related state operations including initialization, the editing flow (populating from getLiteLLMModelAlias), saving operations, and cancel/reset paths. The props passed to LiteLLMConfigDialog are wired to the newly named state accordingly. The observable data flow and external behavior remain unchanged; only the internal naming and state wiring are modified.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e46c8d4 and 7b36155.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • client/src/components/SettingsTab.tsx (3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Prefer interfaces for defining object shapes
Constrain generics with appropriate type bounds
Use type guards to narrow unknown or union types before usage
Enforce import ordering consistently

**/*.{ts,tsx}: Prefer named exports in TypeScript modules
Use 2-space indentation
Declare types and interfaces using PascalCase

Files:

  • client/src/components/SettingsTab.tsx
client/src/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Follow consistent React component structure in the frontend

Files:

  • client/src/components/SettingsTab.tsx
client/src/{app,components}/**/*.tsx

📄 CodeRabbit inference engine (client/CLAUDE.md)

client/src/{app,components}/**/*.tsx: Use React functional components with React.FC typing for all UI components and pages
Define explicit Props interfaces for components, including proper children prop handling
Type event handlers with React-provided types (e.g., React.MouseEvent, FormEvent)
Follow React 19 patterns: hooks-centric components, Suspense and Error Boundaries where appropriate, and concurrent features
Maintain component isolation: avoid excessive prop drilling, use Context wisely, favor composition, and optimize renders
Build responsive layouts using the established Tailwind breakpoint system, grids, flex, and container queries
Ensure accessibility: proper ARIA attributes, keyboard navigation, focus management, and screen reader support

Files:

  • client/src/components/SettingsTab.tsx
client/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (client/CLAUDE.md)

Implement strict TypeScript types: strict prop types, event types, state interfaces, and utility types

In client code, use the @/ alias for imports

Files:

  • client/src/components/SettingsTab.tsx
client/src/{app,components,hooks}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (client/CLAUDE.md)

Use React hooks for local state: apply useState/useReducer patterns, create custom hooks, and always clean up effects

Files:

  • client/src/components/SettingsTab.tsx
client/src/components/**/*.tsx

📄 CodeRabbit inference engine (client/CLAUDE.md)

Use Radix UI primitives for dialogs, dropdowns, form controls, and tooltips

React components in client/src/components should be functional and saved as PascalCase.tsx files

Files:

  • client/src/components/SettingsTab.tsx
🔇 Additional comments (5)
client/src/components/SettingsTab.tsx (5)

45-45: Excellent naming improvement.

The liteLLMModelAliasInput name achieves three goals: consistent capitalization with the hook functions, clear distinction as local input state, and alignment with the established pattern for other provider configurations.


153-153: State initialization correctly implemented.

Populating the input from getLiteLLMModelAlias() when editing starts ensures the dialog displays the current value, maintaining consistency with the Ollama and OpenRouter flows.


160-164: Save flow properly structured.

The persistence of liteLLMModelAliasInput via setLiteLLMModelAlias followed by cleanup maintains the separation between transient input state and persistent storage, exactly as implemented for other providers.


171-171: Cancel cleanup correctly implemented.

Resetting liteLLMModelAliasInput on cancel prevents uncommitted edits from persisting in the component state, consistent with the cancellation behavior for other configurations.


252-255: Dialog props correctly wired and verified.

The props at lines 252–255 match the LiteLLMConfigDialogProps interface exactly. modelAlias receives a string state, and onModelAliasChange receives the appropriate setter callback. The refactoring is complete and consistent with other configuration dialogs.


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

@Agastya18
Copy link
Contributor Author

@chelojimenez

Copy link
Collaborator

@matteo8p matteo8p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 4, 2025
@khandrew1
Copy link
Collaborator

Hey there! Can you resolve the merge conflicts on this PR so we can merge it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LiteLLM Variable rename