fix: subscription auth uses api_key, base URL optional for cloud providers#915
fix: subscription auth uses api_key, base URL optional for cloud providers#915
Conversation
…iders Subscription tokens were passed as auth_token to LiteLLM, which is not a litellm.completion() parameter and was silently discarded. Pass as api_key instead -- LiteLLM auto-detects OAuth tokens by prefix (sk-ant-oat for Anthropic) and switches to Authorization: Bearer. Add requires_base_url field to ProviderPreset so the form correctly shows base URL as optional for cloud providers (Anthropic, OpenAI, etc.) and required only for Azure and self-hosted backends. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove vendor names from driver comments and test docstrings - Add allow_inf_nan=False to ProviderPreset ConfigDict - Enforce requires_base_url server-side in create_from_preset - Disable form submit when requires_base_url and no URL provided - Fix remaining auth_token reference in operations.md prose - Document requires_base_url in design spec presets section - Add OpenRouter to preset base URL test coverage - Include PresetPicker.stories.tsx in commit Pre-reviewed by 7 agents, 11 findings addressed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughAdds Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Code Review
This pull request corrects the handling of subscription tokens by passing them as api_key to LiteLLM and introduces a requires_base_url property for provider presets to enforce mandatory base URLs for self-hosted and Azure backends. The changes include server-side validation during provider creation and UI updates to provide hints and disable submission when a required URL is missing. Feedback indicates that the validation logic is currently missing from the provider update flow and that the frontend hints do not correctly handle edit mode, potentially allowing invalid configurations to be saved.
| if preset.requires_base_url and not base_url: | ||
| msg = ( | ||
| f"Preset {preset.name!r} requires a base URL -- " | ||
| "provide one via base_url" | ||
| ) | ||
| logger.warning( | ||
| PROVIDER_VALIDATION_FAILED, | ||
| preset=request.preset_name, | ||
| error=msg, | ||
| ) | ||
| raise ProviderValidationError(msg) |
There was a problem hiding this comment.
This is a great server-side validation for creating providers from presets.
However, a similar validation appears to be missing in the update_provider flow. A user could edit a provider that requires a base URL (like one created from an Azure or Ollama preset) and remove the URL, which would break the provider.
To ensure consistency and prevent invalid configurations, consider adding a similar validation to the update logic. A robust way to achieve this would be to persist the requires_base_url flag within the ProviderConfig itself.
| isCustom || mode === 'edit' ? undefined | ||
| : preset && !preset.default_base_url ? 'Required for this provider' | ||
| : preset?.requires_base_url ? 'Required for this provider' | ||
| : preset ? 'Optional -- override the default endpoint' | ||
| : undefined |
There was a problem hiding this comment.
This logic for baseUrlHint works well for creation mode, but it doesn't seem to cover edit mode. In edit mode, preset is undefined, so the hint is always undefined.
This means a user editing a provider that requires a base URL won't see the 'Required for this provider' hint if they clear the field.
A similar issue exists for the submit button's disabled logic on line 385, which could allow a user to save a provider without a required base URL.
While the root cause seems to be that the ProviderConfig doesn't store whether a base URL is required, it would be good to see if this can be handled on the frontend as well, perhaps by inferring the requirement from the provider being edited, if possible.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/providers/presets.py`:
- Around line 32-35: The docstring for the parameters requires_base_url and
candidate_urls contains real vendor names; update the phrasing to remove
vendor/vendor-product names and use generic terms instead (e.g., replace
"LiteLLM knows the URL" with "the library/provider knows the URL" and replace
"Azure (per-deployment)" with "deployment-specific cloud backends
(per-deployment)"). Edit the docstring in src/synthorg/providers/presets.py
around the requires_base_url and candidate_urls descriptions to use these
generic terms.
In `@tests/unit/providers/management/test_service.py`:
- Around line 255-266: The test uses a vendor-specific preset name ("azure");
replace it with a synthetic preset and generic names: import ProviderPreset from
synthorg.providers.presets, construct or use a synthetic ProviderPreset with
name "test-provider" (and requires_base_url=True) and pass that preset's name
into CreateFromPresetRequest in test_create_from_preset_requires_base_url (or
use a test fixture that yields such a ProviderPreset), keeping
CreateFromPresetRequest, ProviderManagementService, and the pytest.raises
assertion unchanged.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6544efb6-8590-4e11-befc-2eced7e6ff5e
📒 Files selected for processing (10)
docs/design/operations.mdsrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/management/service.pysrc/synthorg/providers/presets.pytests/unit/providers/drivers/test_litellm_auth.pytests/unit/providers/management/test_service.pytests/unit/providers/test_presets.pyweb/src/api/types.tsweb/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Analyze (python)
- GitHub Check: Dependency Review
🧰 Additional context used
📓 Path-based instructions (12)
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{ts,tsx}: Web Dashboard: ALWAYS reuse existing components fromweb/src/components/ui/before creating new ones. Reuse StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle.
Do NOT recreate status dots inline—use<StatusBadge>. Do NOT build card-with-header layouts from scratch—use<SectionCard>. Do NOT create metric displays withtext-metric font-bold—use<MetricCard>. Do NOT render initials circles manually—use<Avatar>. Do NOT create complex (>8 line) JSX inside.map()—extract to a shared component.
Web design system enforcement: A PostToolUse hook (scripts/check_web_design_system.py) runs automatically on every Edit/Write toweb/src/files. It catches hardcoded hex colors, rgba values, font-family declarations, missing Storybook stories, duplicate patterns, and complex map blocks. Fix all violations before proceeding.
React 19 + shadcn/ui + Tailwind CSS 4 dashboard: Use React Router, Zustand stores,@tanstack/react-query,@xyflow/react,@dagrejs/dagre, d3-force,@dnd-kit, Recharts, Framer Motion, cmdk, js-yaml, Axios.
Files:
web/src/api/types.tsweb/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsx
web/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Design tokens: Colors use Tailwind semantic classes (
text-foreground,bg-card,text-accent, etc.) or CSS variables (var(--so-accent)). NEVER hardcode hex values. Typography usesfont-sansorfont-mono. Spacing uses density-aware tokens or standard Tailwind. Shadows/borders use token variables.
Files:
web/src/api/types.tsweb/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsx
**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
TypeScript 6.0: Remove
baseUrl(deprecated);pathsentries are relative to tsconfig directory.esModuleInteropalways true—cannot be set tofalse.typesdefaults to[]—must explicitly list (e.g.'types': ['vitest/globals']).DOM.Iterablemerged intoDOM.moduleResolution: 'classic'and'node10'removed—use'bundler'or'nodenext'.strictdefaults totrue.noUncheckedSideEffectImportsdefaults totrue.
Files:
web/src/api/types.tsweb/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsx
**/*.{py,ts,tsx,go}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{py,ts,tsx,go}: Lint & format: Python via ruff (uv run ruff check+uv run ruff format). TypeScript web via ESLint (zero warnings enforced:npm --prefix web run lint). Go CLI via golangci-lint andgo vet. All enforced in pre-commit hooks.
Type checking: Python via mypy strict mode (uv run mypy src/ tests/). TypeScript vianpm --prefix web run type-check. Go viago -C cli vet ./....
Files:
web/src/api/types.tstests/unit/providers/management/test_service.pysrc/synthorg/providers/drivers/litellm_driver.pyweb/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsxtests/unit/providers/test_presets.pysrc/synthorg/providers/presets.pytests/unit/providers/drivers/test_litellm_auth.pysrc/synthorg/providers/management/service.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Python version 3.14+ with PEP 649 native lazy annotations; use PEP 758 except syntax (except A, B:without parentheses). Do NOT usefrom __future__ import annotations.
Type hints: all public functions must have type hints. Use mypy strict mode for type checking.
Docstrings: Google style is required on all public classes and functions. Enforced by ruff D rules.
Immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Config vs runtime state: use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Pydantic v2: useBaseModel,model_validator,computed_field,ConfigDict. Adopt conventions: useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Inf; use@computed_fieldfor derived values; useNotBlankStrfor all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
Async concurrency: preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.
Line length: 88 characters enforced by ruff.
Functions must be < 50 lines, files < 800 lines.
Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or co...
Files:
tests/unit/providers/management/test_service.pysrc/synthorg/providers/drivers/litellm_driver.pytests/unit/providers/test_presets.pysrc/synthorg/providers/presets.pytests/unit/providers/drivers/test_litellm_auth.pysrc/synthorg/providers/management/service.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow. Coverage: 80% minimum enforced in CI.
Async testing:asyncio_mode = 'auto'inpyproject.toml—no manual@pytest.mark.asyncioneeded.
Test timeout: 30 seconds per test (global inpyproject.toml)—do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed.
Prefer@pytest.mark.parametrizefor testing similar cases in Python tests.
Property-based testing: Python uses Hypothesis (@given+@settings). Profiles:ci(50 examples, default) anddev(1000 examples). Run dev profile:HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
NEVER skip, dismiss, or ignore flaky tests—always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep(). For tasks that must block indefinitely, useasyncio.Event().wait()instead ofasyncio.sleep(large_number).
Testing: Python pytest with markers (@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e) and coverage >=80%. Web dashboard Vitest with coverage scoped to files changed vs origin/main. Gogo -C cli test ./....
Files:
tests/unit/providers/management/test_service.pytests/unit/providers/test_presets.pytests/unit/providers/drivers/test_litellm_auth.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__). Never useimport logging/logging.getLogger()/print()in application code.
Always use constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api). Import directly and use structured kwargs:logger.info(EVENT, key=value). Never uselogger.info('msg %s', val).
All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Pure data models, enums, and re-exports do NOT need logging.
Files:
src/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/presets.pysrc/synthorg/providers/management/service.py
src/synthorg/providers/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/providers/**/*.py: All provider calls go throughBaseCompletionProviderwhich applies retry + rate limiting automatically. Never implement retry logic in driver subclasses or calling code.
RetryConfigandRateLimiterConfigare set per-provider inProviderConfig. Retryable errors:RateLimitError,ProviderTimeoutError,ProviderConnectionError,ProviderInternalError. Non-retryable errors raise immediately.RetryExhaustedErrorsignals all retries failed.
Rate limiter respectsRateLimitError.retry_afterfrom providers—automatically pauses future requests.
Providers: LLM provider abstraction, presets, model auto-discovery, runtime CRUD management, provider families, discovery SSRF allowlist, health tracking. Apply retry + rate limiting automatically viaBaseCompletionProvider.
Files:
src/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/presets.pysrc/synthorg/providers/management/service.py
web/**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Storybook 10: ESM-only—no CJS support. Packages like
@storybook/addon-essentials,@storybook/addon-interactions,@storybook/test,@storybook/blocksno longer published.@storybook/addon-docsis separate. Import fromstorybook/test(not@storybook/test),storybook/actions. UsedefineMainfrom@storybook/react-vite/nodeanddefinePreviewfrom@storybook/react-vite. Backgrounds API: useparameters.backgrounds.options(object) +initialGlobals.backgrounds.value. a11y testing: useparameters.a11y.test: 'error' | 'todo' | 'off'. Minimum versions: Node 20.19+, Vite 5+, Vitest 3+, TypeScript 4.9+.
Files:
web/src/pages/providers/PresetPicker.stories.tsx
web/src/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Web dashboard pages: Lazy-loaded page components (one per route). Page-scoped sub-components in
pages/<page-name>/subdirs. Feature dirs added as pages are built. Stores for auth, WebSocket, toast, analytics, setup wizard, company, agents, budget, tasks, settings, providers, theme.
Files:
web/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsx
docs/design/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When approved deviations occur, update the relevant
docs/design/page to reflect the new reality.
Files:
docs/design/operations.md
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Documentation:
docs/(Markdown, built with Zensical, config:mkdocs.yml). Design spec indocs/design/(11 pages), architecture indocs/architecture/, roadmap indocs/roadmap/. REST API reference auto-generated byscripts/export_openapi.py. Library reference auto-generated via mkdocstrings + Griffe. Landing page insite/(Astro).
Files:
docs/design/operations.md
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
📚 Learning: 2026-03-29T11:41:44.483Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-29T11:41:44.483Z
Learning: Applies to **/*.{ts,tsx,json} : TypeScript 6.0: Remove `baseUrl` (deprecated); `paths` entries are relative to tsconfig directory. `esModuleInterop` always true—cannot be set to `false`. `types` defaults to `[]`—must explicitly list (e.g. `'types': ['vitest/globals']`). `DOM.Iterable` merged into `DOM`. `moduleResolution: 'classic'` and `'node10'` removed—use `'bundler'` or `'nodenext'`. `strict` defaults to `true`. `noUncheckedSideEffectImports` defaults to `true`.
Applied to files:
web/src/api/types.tsweb/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsx
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Applied to files:
src/synthorg/providers/drivers/litellm_driver.pytests/unit/providers/test_presets.pysrc/synthorg/providers/presets.pydocs/design/operations.mdsrc/synthorg/providers/management/service.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/api/**/*.py : Use Litestar for REST API and WebSocket API with JWT + API key + WS ticket authentication, RFC 9457 structured errors, and content negotiation.
Applied to files:
src/synthorg/providers/drivers/litellm_driver.py
📚 Learning: 2026-03-29T11:41:44.483Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-29T11:41:44.483Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: ESM-only—no CJS support. Packages like `storybook/addon-essentials`, `storybook/addon-interactions`, `storybook/test`, `storybook/blocks` no longer published. `storybook/addon-docs` is separate. Import from `storybook/test` (not `storybook/test`), `storybook/actions`. Use `defineMain` from `storybook/react-vite/node` and `definePreview` from `storybook/react-vite`. Backgrounds API: use `parameters.backgrounds.options` (object) + `initialGlobals.backgrounds.value`. a11y testing: use `parameters.a11y.test: 'error' | 'todo' | 'off'`. Minimum versions: Node 20.19+, Vite 5+, Vitest 3+, TypeScript 4.9+.
Applied to files:
web/src/pages/providers/PresetPicker.stories.tsx
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.
Applied to files:
tests/unit/providers/test_presets.py
📚 Learning: 2026-03-29T11:41:44.483Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-29T11:41:44.483Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction, presets, model auto-discovery, runtime CRUD management, provider families, discovery SSRF allowlist, health tracking. Apply retry + rate limiting automatically via `BaseCompletionProvider`.
Applied to files:
src/synthorg/providers/presets.pydocs/design/operations.mdsrc/synthorg/providers/management/service.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/providers/presets.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 BaseModel, model_validator, computed_field, ConfigDict.
Applied to files:
src/synthorg/providers/presets.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
docs/design/operations.md
🪛 LanguageTool
docs/design/operations.md
[style] ~121-~121: Try moving the adverb to make the sentence clearer.
Context: ..., Ollama, LM Studio, vLLM) which the UI uses to conditionally require a base URL. - Preset auto-probe: `POST /ap...
(SPLIT_INFINITIVE)
🔇 Additional comments (10)
src/synthorg/providers/drivers/litellm_driver.py (1)
354-360: Correct fix for subscription token handoff.Passing
subscription_tokenasapi_keyhere is the right behavior for LiteLLM, and this closes the silent-drop issue cleanly.docs/design/operations.md (1)
50-50: Docs are aligned with implementation changes.The updated auth and preset-base-URL descriptions are consistent with the backend and UI behavior introduced in this PR.
Also applies to: 121-121, 124-124
src/synthorg/providers/presets.py (1)
40-40:requires_base_urlmodeling looks correct.The new field default plus targeted preset overrides are consistent with the intended cloud vs. deployment-specific behavior.
Also applies to: 53-53, 199-199, 214-214, 232-232, 250-250
web/src/api/types.ts (1)
772-772: Type surface update is correct.Adding
requires_base_urlhere keeps frontend typing in sync with the preset payload shape.web/src/pages/providers/PresetPicker.stories.tsx (1)
7-9: Story fixtures are correctly updated.Including
requires_base_urlin sample preset objects keeps story data aligned with the current frontend contract.src/synthorg/providers/management/service.py (1)
397-407: Good server-side enforcement for required base URLs.This is the right place to enforce preset URL requirements, and the warning log before raising keeps failure paths observable.
web/src/pages/providers/ProviderFormModal.tsx (1)
138-139: UI gating and hinting are now consistent with preset metadata.Using
requires_base_urlfor both hint text and submit disablement is a solid UX/API-aligned change.Also applies to: 385-385
tests/unit/providers/test_presets.py (1)
76-106: LGTM!Both tests are well-structured with clear docstrings explaining the rationale. The parameterized assertions include descriptive error messages, and the preset categorization (cloud vs. self-hosted/Azure) aligns with the PR's objective of making base URL optional for cloud providers.
tests/unit/providers/drivers/test_litellm_auth.py (2)
124-140: LGTM!The test correctly validates that subscription tokens are passed via
api_key(which LiteLLM recognizes) rather thanauth_token(which was silently discarded). The explicit assertion thatauth_tokenis absent provides good regression protection.
142-156: LGTM!The use of
object.__setattr__to bypass Pydantic's frozen model is well-documented. The comment clearly explains whymodel_copy(update=...)cannot be used here (the validator rejectsNonefor subscription auth), making this a pragmatic approach to test the defensive code path.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #915 +/- ##
=======================================
Coverage 92.15% 92.15%
=======================================
Files 596 596
Lines 31536 31541 +5
Branches 3058 3059 +1
=======================================
+ Hits 29063 29068 +5
Misses 1950 1950
Partials 523 523 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Remove vendor names from ProviderPreset docstring and Storybook stories - Use synthetic preset in test_create_from_preset_requires_base_url - Sanitize error in ProviderFormModal catch block (no credential reflection) - Tighten OAuth auto-detect comments to avoid Anthropic-specific claims - Convert requires_base_url tests to @pytest.mark.parametrize with exhaustiveness check - Replace vendor names in LiteLLM provider placeholder text Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/pages/providers/ProviderFormModal.tsx`:
- Line 386: The submit guard is fine but the payload builders still pass
whitespace-only baseUrl because they use baseUrl || undefined; update
ProviderFormModal to normalize baseUrl before building requests (e.g., compute a
local const trimmedBaseUrl = baseUrl?.trim(); and use trimmedBaseUrl ?
trimmedBaseUrl : undefined wherever baseUrl is included in the payload at the
spots referenced around the current uses at lines that call baseUrl ||
undefined), ensuring all payloads use the trimmed value or undefined so
whitespace-only URLs are not sent to the backend.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 27dfd3fe-8321-45d5-9f50-294614badf90
📒 Files selected for processing (8)
docs/design/operations.mdsrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/presets.pytests/unit/providers/drivers/test_litellm_auth.pytests/unit/providers/management/test_service.pytests/unit/providers/test_presets.pyweb/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Python version 3.14+ with PEP 649 native lazy annotations; use PEP 758 except syntax (except A, B:without parentheses). Do NOT usefrom __future__ import annotations.
Type hints: all public functions must have type hints. Use mypy strict mode for type checking.
Docstrings: Google style is required on all public classes and functions. Enforced by ruff D rules.
Immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Config vs runtime state: use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Pydantic v2: useBaseModel,model_validator,computed_field,ConfigDict. Adopt conventions: useallow_inf_nan=Falsein allConfigDictdeclarations to rejectNaN/Inf; use@computed_fieldfor derived values; useNotBlankStrfor all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
Async concurrency: preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.
Line length: 88 characters enforced by ruff.
Functions must be < 50 lines, files < 800 lines.
Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or co...
Files:
src/synthorg/providers/drivers/litellm_driver.pytests/unit/providers/management/test_service.pytests/unit/providers/test_presets.pytests/unit/providers/drivers/test_litellm_auth.pysrc/synthorg/providers/presets.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__). Never useimport logging/logging.getLogger()/print()in application code.
Always use constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api). Import directly and use structured kwargs:logger.info(EVENT, key=value). Never uselogger.info('msg %s', val).
All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Pure data models, enums, and re-exports do NOT need logging.
Files:
src/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/presets.py
src/synthorg/providers/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/providers/**/*.py: All provider calls go throughBaseCompletionProviderwhich applies retry + rate limiting automatically. Never implement retry logic in driver subclasses or calling code.
RetryConfigandRateLimiterConfigare set per-provider inProviderConfig. Retryable errors:RateLimitError,ProviderTimeoutError,ProviderConnectionError,ProviderInternalError. Non-retryable errors raise immediately.RetryExhaustedErrorsignals all retries failed.
Rate limiter respectsRateLimitError.retry_afterfrom providers—automatically pauses future requests.
Providers: LLM provider abstraction, presets, model auto-discovery, runtime CRUD management, provider families, discovery SSRF allowlist, health tracking. Apply retry + rate limiting automatically viaBaseCompletionProvider.
Files:
src/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/presets.py
**/*.{py,ts,tsx,go}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{py,ts,tsx,go}: Lint & format: Python via ruff (uv run ruff check+uv run ruff format). TypeScript web via ESLint (zero warnings enforced:npm --prefix web run lint). Go CLI via golangci-lint andgo vet. All enforced in pre-commit hooks.
Type checking: Python via mypy strict mode (uv run mypy src/ tests/). TypeScript vianpm --prefix web run type-check. Go viago -C cli vet ./....
Files:
src/synthorg/providers/drivers/litellm_driver.pytests/unit/providers/management/test_service.pytests/unit/providers/test_presets.pyweb/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsxtests/unit/providers/drivers/test_litellm_auth.pysrc/synthorg/providers/presets.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow. Coverage: 80% minimum enforced in CI.
Async testing:asyncio_mode = 'auto'inpyproject.toml—no manual@pytest.mark.asyncioneeded.
Test timeout: 30 seconds per test (global inpyproject.toml)—do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed.
Prefer@pytest.mark.parametrizefor testing similar cases in Python tests.
Property-based testing: Python uses Hypothesis (@given+@settings). Profiles:ci(50 examples, default) anddev(1000 examples). Run dev profile:HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
NEVER skip, dismiss, or ignore flaky tests—always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep(). For tasks that must block indefinitely, useasyncio.Event().wait()instead ofasyncio.sleep(large_number).
Testing: Python pytest with markers (@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e) and coverage >=80%. Web dashboard Vitest with coverage scoped to files changed vs origin/main. Gogo -C cli test ./....
Files:
tests/unit/providers/management/test_service.pytests/unit/providers/test_presets.pytests/unit/providers/drivers/test_litellm_auth.py
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{ts,tsx}: Web Dashboard: ALWAYS reuse existing components fromweb/src/components/ui/before creating new ones. Reuse StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle.
Do NOT recreate status dots inline—use<StatusBadge>. Do NOT build card-with-header layouts from scratch—use<SectionCard>. Do NOT create metric displays withtext-metric font-bold—use<MetricCard>. Do NOT render initials circles manually—use<Avatar>. Do NOT create complex (>8 line) JSX inside.map()—extract to a shared component.
Web design system enforcement: A PostToolUse hook (scripts/check_web_design_system.py) runs automatically on every Edit/Write toweb/src/files. It catches hardcoded hex colors, rgba values, font-family declarations, missing Storybook stories, duplicate patterns, and complex map blocks. Fix all violations before proceeding.
React 19 + shadcn/ui + Tailwind CSS 4 dashboard: Use React Router, Zustand stores,@tanstack/react-query,@xyflow/react,@dagrejs/dagre, d3-force,@dnd-kit, Recharts, Framer Motion, cmdk, js-yaml, Axios.
Files:
web/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsx
web/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Design tokens: Colors use Tailwind semantic classes (
text-foreground,bg-card,text-accent, etc.) or CSS variables (var(--so-accent)). NEVER hardcode hex values. Typography usesfont-sansorfont-mono. Spacing uses density-aware tokens or standard Tailwind. Shadows/borders use token variables.
Files:
web/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsx
**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
TypeScript 6.0: Remove
baseUrl(deprecated);pathsentries are relative to tsconfig directory.esModuleInteropalways true—cannot be set tofalse.typesdefaults to[]—must explicitly list (e.g.'types': ['vitest/globals']).DOM.Iterablemerged intoDOM.moduleResolution: 'classic'and'node10'removed—use'bundler'or'nodenext'.strictdefaults totrue.noUncheckedSideEffectImportsdefaults totrue.
Files:
web/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsx
web/**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Storybook 10: ESM-only—no CJS support. Packages like
@storybook/addon-essentials,@storybook/addon-interactions,@storybook/test,@storybook/blocksno longer published.@storybook/addon-docsis separate. Import fromstorybook/test(not@storybook/test),storybook/actions. UsedefineMainfrom@storybook/react-vite/nodeanddefinePreviewfrom@storybook/react-vite. Backgrounds API: useparameters.backgrounds.options(object) +initialGlobals.backgrounds.value. a11y testing: useparameters.a11y.test: 'error' | 'todo' | 'off'. Minimum versions: Node 20.19+, Vite 5+, Vitest 3+, TypeScript 4.9+.
Files:
web/src/pages/providers/PresetPicker.stories.tsx
web/src/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Web dashboard pages: Lazy-loaded page components (one per route). Page-scoped sub-components in
pages/<page-name>/subdirs. Feature dirs added as pages are built. Stores for auth, WebSocket, toast, analytics, setup wizard, company, agents, budget, tasks, settings, providers, theme.
Files:
web/src/pages/providers/PresetPicker.stories.tsxweb/src/pages/providers/ProviderFormModal.tsx
docs/design/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When approved deviations occur, update the relevant
docs/design/page to reflect the new reality.
Files:
docs/design/operations.md
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Documentation:
docs/(Markdown, built with Zensical, config:mkdocs.yml). Design spec indocs/design/(11 pages), architecture indocs/architecture/, roadmap indocs/roadmap/. REST API reference auto-generated byscripts/export_openapi.py. Library reference auto-generated via mkdocstrings + Griffe. Landing page insite/(Astro).
Files:
docs/design/operations.md
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-29T11:41:44.483Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction, presets, model auto-discovery, runtime CRUD management, provider families, discovery SSRF allowlist, health tracking. Apply retry + rate limiting automatically via `BaseCompletionProvider`.
📚 Learning: 2026-03-20T08:28:32.845Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Applied to files:
src/synthorg/providers/drivers/litellm_driver.pytests/unit/providers/management/test_service.pytests/unit/providers/test_presets.pydocs/design/operations.mdsrc/synthorg/providers/presets.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/api/**/*.py : Use Litestar for REST API and WebSocket API with JWT + API key + WS ticket authentication, RFC 9457 structured errors, and content negotiation.
Applied to files:
src/synthorg/providers/drivers/litellm_driver.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to tests/**/*.py : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.
Applied to files:
tests/unit/providers/management/test_service.pytests/unit/providers/test_presets.py
📚 Learning: 2026-03-29T11:41:44.483Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-29T11:41:44.483Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: ESM-only—no CJS support. Packages like `storybook/addon-essentials`, `storybook/addon-interactions`, `storybook/test`, `storybook/blocks` no longer published. `storybook/addon-docs` is separate. Import from `storybook/test` (not `storybook/test`), `storybook/actions`. Use `defineMain` from `storybook/react-vite/node` and `definePreview` from `storybook/react-vite`. Backgrounds API: use `parameters.backgrounds.options` (object) + `initialGlobals.backgrounds.value`. a11y testing: use `parameters.a11y.test: 'error' | 'todo' | 'off'`. Minimum versions: Node 20.19+, Vite 5+, Vitest 3+, TypeScript 4.9+.
Applied to files:
web/src/pages/providers/PresetPicker.stories.tsx
📚 Learning: 2026-03-29T11:41:44.483Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-29T11:41:44.483Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction, presets, model auto-discovery, runtime CRUD management, provider families, discovery SSRF allowlist, health tracking. Apply retry + rate limiting automatically via `BaseCompletionProvider`.
Applied to files:
docs/design/operations.mdsrc/synthorg/providers/presets.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Use Pydantic v2 conventions: `BaseModel`, `model_validator`, `computed_field`, `ConfigDict`. For derived values use `computed_field` instead of storing + validating redundant fields. Use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/providers/presets.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to {src,tests,web,cli,site}/**/*.{py,ts,tsx,go,astro} : Vendor-agnostic everywhere: NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names: example-provider, example-large-001, example-medium-001, example-small-001. Vendor names may only appear in: (1) Operations design page provider list (docs/design/operations.md), (2) .claude/ skill/agent files, (3) third-party import paths/module names.
Applied to files:
src/synthorg/providers/presets.py
📚 Learning: 2026-03-29T11:41:44.483Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-29T11:41:44.483Z
Learning: Applies to **/*.py : NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names: `example-provider`, `example-large-001`, `example-medium-001`, `example-small-001`, `large`/`medium`/`small` as aliases. Tests must use `test-provider`, `test-small-001`, etc.
Applied to files:
src/synthorg/providers/presets.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to web/**/*.vue : Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in dashboard code — use generic names: `example-provider`, `example-large-001`, etc.
Applied to files:
src/synthorg/providers/presets.py
🪛 LanguageTool
docs/design/operations.md
[style] ~121-~121: Try moving the adverb to make the sentence clearer.
Context: ..., Ollama, LM Studio, vLLM) which the UI uses to conditionally require a base URL. - Preset auto-probe: `POST /ap...
(SPLIT_INFINITIVE)
🔇 Additional comments (8)
src/synthorg/providers/drivers/litellm_driver.py (1)
354-359: Subscription token handoff fix is correct.Mapping
subscription_tokentoapi_keyhere eliminates the silent no-op behavior from unsupported kwargs and aligns auth handling with the driver contract.docs/design/operations.md (1)
50-50: Docs are aligned with runtime behavior.These updates correctly document both the subscription token
api_keyhandoff and the new presetrequires_base_urlflow.Also applies to: 121-121, 124-124
web/src/pages/providers/PresetPicker.stories.tsx (1)
7-9: Story fixtures are correctly updated for the new preset schema.Including
requires_base_urland switching to generic example presets keeps the stories type-safe and policy-aligned.Also applies to: 27-27
tests/unit/providers/management/test_service.py (1)
255-284: Good boundary test for required base URL validation.This test precisely covers the
requires_base_url=Truerejection path with a synthetic preset and a clear assertion target.tests/unit/providers/test_presets.py (1)
76-112: Preset categorization tests are solid and future-proof.The parametric cloud/self-hosted assertions plus the exhaustiveness check give good guardrails for future preset additions.
web/src/pages/providers/ProviderFormModal.tsx (1)
138-140: Nice UX and safety improvements in the form copy/behavior.The contextual base URL hint, safer error-message extraction, and generic LiteLLM placeholder are all good updates.
Also applies to: 226-227, 372-372
src/synthorg/providers/presets.py (1)
41-55:requires_base_urlintegration is clean and consistent.Schema hardening (
allow_inf_nan=False) and preset-levelrequires_base_urlflags are well applied across the updated definitions.Also applies to: 190-251
tests/unit/providers/drivers/test_litellm_auth.py (1)
124-155: Auth tests now correctly enforce the subscription→api_keycontract.Great update to assert
api_keypresence andauth_tokenabsence, plus explicit coverage of the no-token branch.
| <Button | ||
| onClick={handleSubmit} | ||
| disabled={submitting || !name.trim() || (authType === 'subscription' && !tosAccepted)} | ||
| disabled={submitting || !name.trim() || (authType === 'subscription' && !tosAccepted) || (preset?.requires_base_url && !baseUrl.trim())} |
There was a problem hiding this comment.
Normalize baseUrl before building request payloads.
At Line 386 you correctly guard required presets, but optional presets can still send whitespace-only URLs because Line 190/204/219 use baseUrl || undefined. That can trigger avoidable backend validation errors.
💡 Proposed fix
- base_url: baseUrl || undefined,
+ base_url: baseUrl.trim() || undefined,
...
- base_url: baseUrl || undefined,
+ base_url: baseUrl.trim() || undefined,
...
- base_url: baseUrl || undefined,
+ base_url: baseUrl.trim() || undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/providers/ProviderFormModal.tsx` at line 386, The submit guard
is fine but the payload builders still pass whitespace-only baseUrl because they
use baseUrl || undefined; update ProviderFormModal to normalize baseUrl before
building requests (e.g., compute a local const trimmedBaseUrl = baseUrl?.trim();
and use trimmedBaseUrl ? trimmedBaseUrl : undefined wherever baseUrl is included
in the payload at the spots referenced around the current uses at lines that
call baseUrl || undefined), ensuring all payloads use the trimmed value or
undefined so whitespace-only URLs are not sent to the backend.
Compute trimmedBaseUrl once and use it across all three payload builders so whitespace-only URLs are sent as undefined instead of reaching the backend. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/pages/providers/ProviderFormModal.tsx`:
- Around line 227-228: The catch in ProviderFormModal's submit handler currently
extracts only err.message and logs that string, which discards stack and cause
information; update the submit error handling (the catch block that logs
"ProviderFormModal: submit failed:") to log the whole error object (or err.stack
when available) instead of only the message so stack/context is preserved, and
ensure non-Error values are logged as-is; remove the intermediate msg-only
logging and emit the full err alongside a clear context string.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3e42cc4-648d-4de0-a0a0-587fa7a07173
📒 Files selected for processing (1)
web/src/pages/providers/ProviderFormModal.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{ts,tsx}: Web Dashboard: ALWAYS reuse existing components fromweb/src/components/ui/before creating new ones. Reuse StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle.
Do NOT recreate status dots inline—use<StatusBadge>. Do NOT build card-with-header layouts from scratch—use<SectionCard>. Do NOT create metric displays withtext-metric font-bold—use<MetricCard>. Do NOT render initials circles manually—use<Avatar>. Do NOT create complex (>8 line) JSX inside.map()—extract to a shared component.
Web design system enforcement: A PostToolUse hook (scripts/check_web_design_system.py) runs automatically on every Edit/Write toweb/src/files. It catches hardcoded hex colors, rgba values, font-family declarations, missing Storybook stories, duplicate patterns, and complex map blocks. Fix all violations before proceeding.
React 19 + shadcn/ui + Tailwind CSS 4 dashboard: Use React Router, Zustand stores,@tanstack/react-query,@xyflow/react,@dagrejs/dagre, d3-force,@dnd-kit, Recharts, Framer Motion, cmdk, js-yaml, Axios.
Files:
web/src/pages/providers/ProviderFormModal.tsx
web/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Design tokens: Colors use Tailwind semantic classes (
text-foreground,bg-card,text-accent, etc.) or CSS variables (var(--so-accent)). NEVER hardcode hex values. Typography usesfont-sansorfont-mono. Spacing uses density-aware tokens or standard Tailwind. Shadows/borders use token variables.
Files:
web/src/pages/providers/ProviderFormModal.tsx
**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
TypeScript 6.0: Remove
baseUrl(deprecated);pathsentries are relative to tsconfig directory.esModuleInteropalways true—cannot be set tofalse.typesdefaults to[]—must explicitly list (e.g.'types': ['vitest/globals']).DOM.Iterablemerged intoDOM.moduleResolution: 'classic'and'node10'removed—use'bundler'or'nodenext'.strictdefaults totrue.noUncheckedSideEffectImportsdefaults totrue.
Files:
web/src/pages/providers/ProviderFormModal.tsx
**/*.{py,ts,tsx,go}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{py,ts,tsx,go}: Lint & format: Python via ruff (uv run ruff check+uv run ruff format). TypeScript web via ESLint (zero warnings enforced:npm --prefix web run lint). Go CLI via golangci-lint andgo vet. All enforced in pre-commit hooks.
Type checking: Python via mypy strict mode (uv run mypy src/ tests/). TypeScript vianpm --prefix web run type-check. Go viago -C cli vet ./....
Files:
web/src/pages/providers/ProviderFormModal.tsx
web/src/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Web dashboard pages: Lazy-loaded page components (one per route). Page-scoped sub-components in
pages/<page-name>/subdirs. Feature dirs added as pages are built. Stores for auth, WebSocket, toast, analytics, setup wizard, company, agents, budget, tasks, settings, providers, theme.
Files:
web/src/pages/providers/ProviderFormModal.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction (LiteLLM adapter), auth types (api_key/oauth/custom_header/none), presets (PROVIDER_PRESETS), runtime CRUD (ProviderManagementService with asyncio.Lock serialization), hot-reload via AppState swap.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-29T11:41:44.483Z
Learning: Applies to src/synthorg/providers/**/*.py : Providers: LLM provider abstraction, presets, model auto-discovery, runtime CRUD management, provider families, discovery SSRF allowlist, health tracking. Apply retry + rate limiting automatically via `BaseCompletionProvider`.
📚 Learning: 2026-03-29T11:41:44.483Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-29T11:41:44.483Z
Learning: Applies to **/*.{ts,tsx,json} : TypeScript 6.0: Remove `baseUrl` (deprecated); `paths` entries are relative to tsconfig directory. `esModuleInterop` always true—cannot be set to `false`. `types` defaults to `[]`—must explicitly list (e.g. `'types': ['vitest/globals']`). `DOM.Iterable` merged into `DOM`. `moduleResolution: 'classic'` and `'node10'` removed—use `'bundler'` or `'nodenext'`. `strict` defaults to `true`. `noUncheckedSideEffectImports` defaults to `true`.
Applied to files:
web/src/pages/providers/ProviderFormModal.tsx
🔇 Additional comments (4)
web/src/pages/providers/ProviderFormModal.tsx (4)
136-140:requires_base_urlhint logic is now correctly source-of-truth driven.This aligns the UI hint with preset metadata instead of inferring from
default_base_url.
181-220: Base URL normalization is correctly applied across all submit payloads.Using
trimmedBaseUrlfor create-from-preset, create, and update prevents whitespace-only URLs from reaching the API.
373-373: Updated placeholder copy looks good.Clearer examples without implying specific built-in providers.
387-387: Submit disabling rule correctly enforces required base URL presets.The
trim()check is consistent with backend validation semantics and blocks empty/whitespace input early.
| const msg = err instanceof Error ? err.message : 'Unknown error' | ||
| console.error('ProviderFormModal: submit failed:', msg) |
There was a problem hiding this comment.
Preserve error stack/context in submit logging.
Current code logs only a string message, which drops stack/cause details for Error instances and makes debugging harder.
Suggested fix
- const msg = err instanceof Error ? err.message : 'Unknown error'
- console.error('ProviderFormModal: submit failed:', msg)
+ if (err instanceof Error) {
+ console.error('ProviderFormModal: submit failed:', err)
+ } else {
+ console.error('ProviderFormModal: submit failed:', String(err ?? 'Unknown error'))
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/providers/ProviderFormModal.tsx` around lines 227 - 228, The
catch in ProviderFormModal's submit handler currently extracts only err.message
and logs that string, which discards stack and cause information; update the
submit error handling (the catch block that logs "ProviderFormModal: submit
failed:") to log the whole error object (or err.stack when available) instead of
only the message so stack/context is preserved, and ensure non-Error values are
logged as-is; remove the intermediate msg-only logging and emit the full err
alongside a clear context string.
🤖 I have created a release *beep* *boop* --- #MAJOR CHANGES; We got a somewhat working webui :) ## [0.5.0](v0.4.9...v0.5.0) (2026-03-30) ### Features * add analytics trends and budget forecast API endpoints ([#798](#798)) ([16b61f5](16b61f5)) * add department policies to default templates ([#852](#852)) ([7a41548](7a41548)) * add remaining activity event types (task_started, tool_used, delegation, cost_incurred) ([#832](#832)) ([4252fac](4252fac)) * agent performance, activity, and history API endpoints ([#811](#811)) ([9b75c1d](9b75c1d)) * Agent Profiles and Detail pages (biography, career, performance) ([#874](#874)) ([62d7880](62d7880)) * app shell, Storybook, and CI/CD pipeline ([#819](#819)) ([d4dde90](d4dde90)) * Approvals page with risk grouping, urgency indicators, batch actions ([#889](#889)) ([4e9673d](4e9673d)) * Budget Panel page (P&L dashboard, breakdown charts, forecast) ([#890](#890)) ([b63b0f1](b63b0f1)) * build infrastructure layer (API client, auth, WebSocket) ([#815](#815)) ([9f01d3e](9f01d3e)) * CLI global options infrastructure, UI modes, exit codes, env vars ([#891](#891)) ([fef4fc5](fef4fc5)) * CodeMirror editor and theme preferences toggle ([#905](#905), [#807](#807)) ([#909](#909)) ([41fbedc](41fbedc)) * Company page (department/agent management) ([#888](#888)) ([cfb88b0](cfb88b0)) * comprehensive hint coverage across all CLI commands ([#900](#900)) ([937974e](937974e)) * config system extensions, per-command flags for init/start/stop/status/logs ([#895](#895)) ([32f83fe](32f83fe)) * configurable currency system replacing hardcoded USD ([#854](#854)) ([b372551](b372551)) * Dashboard page (metric cards, activity feed, budget burn) ([#861](#861)) ([7d519d5](7d519d5)) * department health, provider status, and activity feed endpoints ([#818](#818)) ([6d5f196](6d5f196)) * design tokens and core UI components ([#833](#833)) ([ed887f2](ed887f2)) * extend approval, meeting, and budget API responses ([#834](#834)) ([31472bf](31472bf)) * frontend polish -- real-time UX, accessibility, responsive, performance ([#790](#790), [#792](#792), [#791](#791), [#793](#793)) ([#917](#917)) ([f04a537](f04a537)) * implement human roles and access control levels ([#856](#856)) ([d6d8a06](d6d8a06)) * implement semantic conflict detection in workspace merge ([#860](#860)) ([d97283b](d97283b)) * interaction components and animation patterns ([#853](#853)) ([82d4b01](82d4b01)) * Login page + first-run bootstrap + Company page ([#789](#789), [#888](#888)) ([#896](#896)) ([8758e8d](8758e8d)) * Meetings page with timeline viz, token bars, contribution formatting ([#788](#788)) ([#904](#904)) ([b207f46](b207f46)) * Messages page with threading, channel badges, sender indicators ([#787](#787)) ([#903](#903)) ([28293ad](28293ad)) * Org Chart force-directed view and drag-drop reassignment ([#872](#872), [#873](#873)) ([#912](#912)) ([a68a938](a68a938)) * Org Chart page (living nodes, status, CRUD, department health) ([#870](#870)) ([0acbdae](0acbdae)) * per-command flags for remaining commands, auto-behavior wiring, help/discoverability ([#897](#897)) ([3f7afa2](3f7afa2)) * Providers page with backend rework -- health, CRUD, subscription auth ([#893](#893)) ([9f8dd98](9f8dd98)) * scaffold React + Vite + TypeScript + Tailwind project ([#799](#799)) ([bd151aa](bd151aa)) * Settings page with search, dependency indicators, grouped rendering ([#784](#784)) ([#902](#902)) ([a7b9870](a7b9870)) * Setup Wizard rebuild with template comparison, cost estimator, theme customization ([#879](#879)) ([ae8b50b](ae8b50b)) * setup wizard UX -- template filters, card metadata, provider form reuse ([#910](#910)) ([7f04676](7f04676)) * setup wizard UX overhaul -- mode choice, step reorder, provider fixes ([#907](#907)) ([ee964c4](ee964c4)) * structured ModelRequirement in template agent configs ([#795](#795)) ([7433548](7433548)) * Task Board page (rich Kanban, filtering, dependency viz) ([#871](#871)) ([04a19b0](04a19b0)) ### Bug Fixes * align frontend types with backend and debounce WS refetches ([#916](#916)) ([134c11b](134c11b)) * auto-cleanup targets newly pulled images instead of old ones ([#884](#884)) ([50e6591](50e6591)) * correct wipe backup-skip flow and harden error handling ([#808](#808)) ([c05860f](c05860f)) * improve provider setup in wizard, subscription auth, dashboard bugs ([#914](#914)) ([87bf8e6](87bf8e6)) * improve update channel detection and add config get command ([#814](#814)) ([6b137f0](6b137f0)) * resolve all ESLint warnings, add zero-warnings enforcement ([#899](#899)) ([079b46a](079b46a)) * subscription auth uses api_key, base URL optional for cloud providers ([#915](#915)) ([f0098dd](f0098dd)) ### Refactoring * semantic analyzer cleanup -- shared filtering, concurrency, extraction ([#908](#908)) ([81372bf](81372bf)) ### Documentation * brand identity and UX design system from [#765](#765) exploration ([#804](#804)) ([389a9f4](389a9f4)) * page structure and information architecture for v0.5.0 dashboard ([#809](#809)) ([f8d6d4a](f8d6d4a)) * write UX design guidelines with WCAG-verified color system ([#816](#816)) ([4a4594e](4a4594e)) ### Tests * add unit tests for agent hooks and page components ([#875](#875)) ([#901](#901)) ([1d81546](1d81546)) ### CI/CD * bump actions/deploy-pages from 4.0.5 to 5.0.0 in the major group ([#831](#831)) ([01c19de](01c19de)) * bump astral-sh/setup-uv from 7.6.0 to 8.0.0 in /.github/actions/setup-python-uv in the all group ([#920](#920)) ([5f6ba54](5f6ba54)) * bump codecov/codecov-action from 5.5.3 to 6.0.0 in the major group ([#868](#868)) ([f22a181](f22a181)) * bump github/codeql-action from 4.34.1 to 4.35.0 in the all group ([#883](#883)) ([87a4890](87a4890)) * bump sigstore/cosign-installer from 4.1.0 to 4.1.1 in the minor-and-patch group ([#830](#830)) ([7a69050](7a69050)) * bump the all group with 3 updates ([#923](#923)) ([ff27c8e](ff27c8e)) * bump wrangler from 4.76.0 to 4.77.0 in /.github in the minor-and-patch group ([#822](#822)) ([07d43eb](07d43eb)) * bump wrangler from 4.77.0 to 4.78.0 in /.github in the all group ([#882](#882)) ([f84118d](f84118d)) ### Maintenance * add design system enforcement hook and component inventory ([#846](#846)) ([15abc43](15abc43)) * add dev-only auth bypass for frontend testing ([#885](#885)) ([6cdcd8a](6cdcd8a)) * add pre-push rebase check hook ([#855](#855)) ([b637a04](b637a04)) * backend hardening -- eviction/size-caps and model validation ([#911](#911)) ([81253d9](81253d9)) * bump axios from 1.13.6 to 1.14.0 in /web in the all group across 1 directory ([#922](#922)) ([b1b0232](b1b0232)) * bump brace-expansion from 5.0.4 to 5.0.5 in /web ([#862](#862)) ([ba4a565](ba4a565)) * bump eslint-plugin-react-refresh from 0.4.26 to 0.5.2 in /web ([#801](#801)) ([7574bb5](7574bb5)) * bump faker from 40.11.0 to 40.11.1 in the minor-and-patch group ([#803](#803)) ([14d322e](14d322e)) * bump https://github.com/astral-sh/ruff-pre-commit from v0.15.7 to 0.15.8 ([#864](#864)) ([f52901e](f52901e)) * bump nginxinc/nginx-unprivileged from `6582a34` to `f99cc61` in /docker/web in the all group ([#919](#919)) ([df85e4f](df85e4f)) * bump nginxinc/nginx-unprivileged from `ccbac1a` to `6582a34` in /docker/web ([#800](#800)) ([f4e9450](f4e9450)) * bump node from `44bcbf4` to `71be405` in /docker/sandbox ([#827](#827)) ([91bec67](91bec67)) * bump node from `5209bca` to `cf38e1f` in /docker/web ([#863](#863)) ([66d6043](66d6043)) * bump picomatch in /site ([#842](#842)) ([5f20bcc](5f20bcc)) * bump recharts 2->3 and @types/node 22->25 in /web ([#802](#802)) ([a908800](a908800)) * Bump requests from 2.32.5 to 2.33.0 ([#843](#843)) ([41daf69](41daf69)) * bump smol-toml from 1.6.0 to 1.6.1 in /site ([#826](#826)) ([3e5dbe4](3e5dbe4)) * bump the all group with 3 updates ([#921](#921)) ([7bace0b](7bace0b)) * bump the minor-and-patch group across 1 directory with 2 updates ([#829](#829)) ([93e611f](93e611f)) * bump the minor-and-patch group across 1 directory with 3 updates ([#841](#841)) ([7010c8e](7010c8e)) * bump the minor-and-patch group across 1 directory with 3 updates ([#869](#869)) ([548cee5](548cee5)) * bump the minor-and-patch group in /site with 2 updates ([#865](#865)) ([9558101](9558101)) * bump the minor-and-patch group with 2 updates ([#867](#867)) ([4830706](4830706)) * consolidate Dependabot groups to 1 PR per ecosystem ([06d2556](06d2556)) * consolidate Dependabot groups to 1 PR per ecosystem ([#881](#881)) ([06d2556](06d2556)) * improve worktree skill with full dep sync and status enhancements ([#906](#906)) ([772c625](772c625)) * remove Vue remnants and document framework decision ([#851](#851)) ([bf2adf6](bf2adf6)) * update web dependencies and fix brace-expansion CVE ([#880](#880)) ([a7a0ed6](a7a0ed6)) * upgrade to Storybook 10 and TypeScript 6 ([#845](#845)) ([52d95f2](52d95f2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
auth_tokento LiteLLM, which is not alitellm.completion()parameter and was silently discarded. Now passed asapi_key-- LiteLLM auto-detects OAuth tokens by prefix and switches toAuthorization: Bearerwith the correct headers.requires_base_urlfield toProviderPresetso cloud providers (Anthropic, OpenAI, etc.) show "Optional -- override the default endpoint" instead of "Required". Only Azure and self-hosted backends show "Required".create_from_presetnow rejects requests missing a base URL when the preset requires one (e.g. Azure).requires_base_url=trueand base URL is empty.Test plan
api_keykwarg (notauth_token)requires_base_urlcategorization for all 11 presetsReview coverage
Pre-reviewed by 7 agents (docs-consistency, code-reviewer, conventions-enforcer, frontend-reviewer, api-contract-drift, type-design-analyzer, pr-test-analyzer). 11 findings addressed.
🤖 Generated with Claude Code