archon-ui-main/src/config/api.ts (1)

34-34: Consider removing the any type cast.

With the proper TypeScript definitions now in place in env.d.ts, the any cast is no longer necessary.

-  const port = (import.meta.env as any).VITE_PORT as string | undefined;
+  const port = import.meta.env.VITE_PORT;

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

.env.example (1)

43-50: Good addition: OPENAI_BASE_URL override is clear and actionable.

This section helps local devs route via a proxy/self-hosted OpenAI-compatible endpoint. One small enhancement: given this PR adds Vite env typings and proxy behavior, include commented Vite overrides to reduce confusion during local setup and make the UI/server behavior explicit.

Apply this diff to add optional Vite overrides (commented) for clarity:

 OPENAI_BASE_URL=http://localhost:8080/v1
 # OPENAI_API_BASE=
 
+# Frontend (Vite) overrides — optional (proxy is used by default)
+# If you need to bypass the Vite proxy for debugging, uncomment:
+# VITE_PORT=3737
+# VITE_API_URL=http://localhost:8181
+

migration/complete_setup.sql (1)

69-75: Optional: keep categories stable across UIs.

The seeding places OPENAI_BASE_URL in 'rag_strategy'. Note the UI’s generic API Keys section currently saves edits under the 'api_keys' category for any key it touches (see APIKeysSection.tsx comment). While the table enforces uniqueness by key (so category can change on update without duplication), consider preserving the original category for consistency and simpler querying.

Also applies to: 89-99

archon-ui-main/src/services/credentialsService.ts (1)

288-307: Minor: avoid PUT-ing unchanged settings; preserve server/category semantics.

updateRagSettings blindly issues a PUT for every key. It works but generates unnecessary writes and log noise. Consider diffing against current values to only update changed keys. Also, ensure server treats category as idempotent for these updates.

I can provide a small helper to fetch current values, diff, and batch only the changes if you’d like.

archon-ui-main/src/components/settings/APIKeysSection.tsx (2)

47-56: Broader filter will include BASE_URL keys (good), but handle category on save.

Expanding the filter to include BASE_URL makes OPENAI_BASE_URL show up here. However, the save path always writes with category 'api_keys' (see below), which can inadvertently move settings seeded under 'rag_strategy'.

Apply the following changes to preserve the original category:

 interface CustomCredential {
   key: string;
   value: string;
   description: string;
   originalValue?: string;
   originalKey?: string; // Track original key for renaming
+  originalCategory?: string; // Preserve category for correct writes
   hasChanges?: boolean;
   is_encrypted?: boolean;
   showValue?: boolean; // Track per-credential visibility
   isNew?: boolean; // Track if this is a new unsaved credential
 }

In loadCredentials mapping:

 const uiCredentials = apiKeys.map(cred => ({
   key: cred.key,
   value: cred.value || '',
   description: cred.description || '',
   originalValue: cred.value || '',
   originalKey: cred.key, // Track original key for updates
+  originalCategory: cred.category,
   hasChanges: false,
   is_encrypted: cred.is_encrypted || false,
   showValue: false,
   isNew: false
 }));

In save paths (both create and update), use the preserved category:

-  category: 'api_keys'
+  category: cred.originalCategory || 'api_keys'

---

149-176: Guard against data loss when toggling encryption off without a new value.

If a credential was encrypted and the user toggles to unencrypted but leaves value empty, the update may clear the stored secret (since plaintext isn’t provided). Consider blocking save for that row until a new plaintext is entered, or require re-entry when flipping encryption.

I can wire a minimal UI validation that tracks original encryption and enforces new plaintext on de-encryption if you want a quick patch.

python/src/server/api_routes/tests_api.py (1)

101-102: Remove unused global active_websockets.

WebSocket connections are fully managed by TestWebSocketManager.connections. The standalone active_websockets dict is never used and can be deleted.

Apply this diff:

-test_executions: dict[str, TestExecution] = {}
-active_websockets: dict[str, list[WebSocket]] = {}
+test_executions: dict[str, TestExecution] = {}

archon-ui-main/src/components/settings/RAGSettings.tsx (2)

76-86: Provider-agnostic naming vs. "Ollama Base URL" label

The "Ollama Base URL" input persists into ragSettings.OPENAI_BASE_URL. This can confuse future readers since the key looks OpenAI-specific but the label and default are Ollama.

Option A (minimal): Update label/help to clarify it's the provider base URL stored in OPENAI_BASE_URL for now.
Option B (cleaner): Rename the prop to BASE_URL across UI/services/docs and have backend map it per provider. If you want, I can prep a follow-up PR plan that minimizes churn.

Also applies to: 79-82

---

89-111: Surface error reason in toast for easier debugging

Currently the error toast is generic. Include error details (safe message) to reduce support loops.

- showToast('Failed to save settings', 'error');
+ const message = (err as any)?.message ?? 'Unknown error';
+ showToast(`Failed to save settings: ${message}`, 'error');

docs/docs/api-reference.mdx (1)

1856-1869: Minor: import placement and API surface in SDK examples

• Python: fine, but consider timeouts/retries in real clients.
• JS: dynamic import inside method is atypical; import at module scope, and prefer socket.disconnect() over close().

Update the JS example import placement to the top and replace any close() with disconnect().

Also applies to: 1988-2021

python/src/server/api_routes/knowledge_api.py (4)

179-216: DELETE handler: message vs. behavior mismatch (item vs. source)

Handler delegates to SourceManagementService.delete_source, then returns “Successfully deleted knowledge item”. Consider aligning the message (“source” vs. “knowledge item”) to reduce confusion when both endpoints exist.

- return {"success": True, "message": f"Successfully deleted knowledge item {source_id}"}
+ return {"success": True, "message": f"Successfully deleted source {source_id}"}

If this path is specifically for items (not the broader source), flip the service call instead to the item-level delete API for consistency.

---

228-259: Code examples lookup: OK — add pagination if this grows

Directly returning full contents can be heavy. If this endpoint sees large payloads, consider limit/offset in the query and a fields selector.

No action required for this PR; just a note for scalability.

---

28-35: Duplicate imports: remove redundant lines

There are repeated imports for get_crawler, RAGService, DocumentStorageService, get_supabase_client.

-from ..services.crawler_manager import get_crawler
-from ..services.search.rag_service import RAGService
-from ..services.storage import DocumentStorageService
-from ..utils import get_supabase_client
+# (duplicates removed; imports already present above)

---

867-877: Prefer timezone-aware timestamps in health response

Use UTC timezone-aware timestamps to avoid ambiguity.

-from datetime import datetime
+from datetime import datetime, timezone
@@
-        "timestamp": datetime.now().isoformat(),
+        "timestamp": datetime.now(timezone.utc).isoformat(),

original_archon/iterations/v5-parallel-specialized-agents/streamlit_pages/environment.py (1)

188-195: Local var named OPENAI_BASE_URL but persisted as BASE_URL

This is just a local name, but it can mislead since the value is provider-agnostic.

Consider renaming the local variable to baseUrl or providerBaseUrl to reflect its usage:

- OPENAI_BASE_URL = st.text_input(
+ base_url = st.text_input(
@@
- updated_values["BASE_URL"] = OPENAI_BASE_URL
+ updated_values["BASE_URL"] = base_url

original_archon/iterations/v6-tool-library-integration/streamlit_pages/environment.py (1)

188-195: Same naming nit: provider-agnostic value stored under BASE_URL

Mirror suggestion from v5 file: rename local OPENAI_BASE_URL to base_url for clarity.

See diff in the v5 comment; apply the same change here.

python/src/server/services/source_management_service.py (5)

19-34: Default model mismatch vs docs; please align MODEL_CHOICE default

Code defaults to "gpt-4.1-nano", while docs (docs/docs/rag.mdx) show "gpt-4o-mini" as the default. Pick one source of truth to avoid operator confusion and flaky expectations. Given tests already reference "gpt-4.1-nano", the quickest fix is to update the docs’ table default. See my docs suggestion on that file.

---

36-118: Duplicate provider/base_url resolution logic; centralize and de-risk Google base URL

This helper largely reimplements logic already present in credential_service/llm_provider_service (e.g., provider/base_url mapping and Google’s OpenAI-compatible endpoint). Risks:

• Divergence over time across places that decide base_url.
• Inconsistent Google base URL path if one site changes; tests expect ".../v1beta/openai/" while docs currently suggest otherwise.

Consider delegating to a small synchronous accessor in credential_service (read from cache only, no I/O) for provider/base_url, so both this module and llm_provider_service share the same single source of truth. At minimum, reuse credential_service._get_provider_base_url to avoid hardcoding URLs here.

Apply at least this logging improvement when falling back to OpenAI on unknown providers:

@@
-        else:
-            # Unknown provider, default to OpenAI without base_url
+        else:
+            # Unknown provider, default to OpenAI without base_url
+            logger.warning(f"Unknown LLM provider '{provider}', defaulting to OpenAI")
             if credential_service._cache_initialized and "OPENAI_API_KEY" in credential_service._cache:

---

162-176: LLM client creation: log with traceback; avoid leaking secrets in messages

Use exc_info=True so we don’t lose the stack on failures.

-            search_logger.error(f"Failed to create LLM client for summary generation: {e}")
+            search_logger.error("Failed to create LLM client for summary generation", exc_info=True)

And similarly in title generation:

-                search_logger.error(
-                    f"Failed to create LLM client for title generation: {e}"
-                )
+                search_logger.error("Failed to create LLM client for title generation", exc_info=True)

Also applies to: 178-179

---

212-215: Keep full stack trace on failure path

-        search_logger.error(
-            f"Error generating summary with LLM for {source_id}: {e}. Using default summary."
-        )
+        search_logger.error(
+            f"Error generating summary with LLM for {source_id}. Using default summary.",
+            exc_info=True,
+        )

---

293-295: Add traceback to title-generation error for postmortem debugging

-            search_logger.error(f"Error generating title for {source_id}: {e}")
+            search_logger.error(f"Error generating title for {source_id}", exc_info=True)

python/src/server/services/crawling/strategies/recursive.py (1)

234-244: Avoid duplicate binary checks and minor micro-optimization

is_binary_file is called twice. Cache it once per link.

-                            if within_scope(next_url):
-                                # Skip binary files and already visited URLs
-                                if next_url not in visited and not self.url_handler.is_binary_file(next_url):
-                                    next_level_urls.add(next_url)
-                                elif self.url_handler.is_binary_file(next_url):
-                                    logger.debug(f"Skipping binary file from crawl queue: {next_url}")
+                            if within_scope(next_url):
+                                is_bin = self.url_handler.is_binary_file(next_url)
+                                # Skip binary files and already visited URLs
+                                if next_url not in visited and not is_bin:
+                                    next_level_urls.add(next_url)
+                                elif is_bin:
+                                    logger.debug(f"Skipping binary file from crawl queue: {next_url}")

python/src/server/services/crawling/crawling_service.py (1)

570-577: Pass restrict_to_start_path explicitly to document intent

Given the strategy now supports path scoping, explicitly opt-in from the orchestrator to avoid surprises if defaults change again.

             crawl_results = await self.crawl_recursive_with_progress(
                 [url],
                 max_depth=max_depth,
                 max_concurrent=None,  # Let strategy use settings
                 progress_callback=await self._create_crawl_progress_callback("crawling"),
                 start_progress=10,
-                end_progress=20,
+                end_progress=20,
+                restrict_to_start_path=True,
             )

python/tests/test_async_llm_provider_service.py (1)

160-183: Optional: add a targeted test for Google’s compatibility base_url

Given the fragile nature of the Google OpenAI-compatible path, consider a small test mirroring the new OpenAI base_url test to lock “.../v1beta/openai/” and prevent regressions.

python/src/server/services/llm_provider_service.py (2)

95-103: OpenAI base_url fallback from credentials is correct; consider adding env fallback here too for symmetry.

In the explicit provider override path, we only consult credentials. In practice, deployments sometimes rely on environment-only overrides (e.g., OPENAI_BASE_URL in Docker). Mirroring the env fallback you added in get_active_provider makes behavior consistent across both paths.

Apply this diff and import:

@@
-            if provider == "openai" and not base_url:
+            if provider == "openai" and not base_url:
                 try:
                     base_url = await credential_service.get_credential("OPENAI_BASE_URL", decrypt=True)
                     if not base_url:
                         base_url = await credential_service.get_credential("OPENAI_API_BASE", decrypt=True)
                 except Exception:
                     base_url = None
+                if not base_url:
+                    # Mirror credential_service.get_active_provider fallbacks
+                    import os
+                    base_url = os.getenv("OPENAI_BASE_URL") or os.getenv("OPENAI_API_BASE")

Optionally, normalize to an OpenAI-compatible path if absent (e.g., append "/v1" when missing). I can provide a small helper if you want that.

---

127-131: Conditional base_url kwarg avoids breaking tests; add richer error logging below.

This change is good. Also, consider emitting the full stack trace in the except block for easier debugging.

Apply within the same function’s except:

-        logger.error(
-            f"Error creating LLM client for provider {provider_name if 'provider_name' in locals() else 'unknown'}: {e}"
-        )
+        logger.error(
+            f"Error creating LLM client for provider {provider_name if 'provider_name' in locals() else 'unknown'}: {e}",
+            exc_info=True,
+        )

python/src/server/services/crawling/document_storage_operations.py (1)

90-93: Using a provided, scoped source_id consistently per crawl is the right call.

The logging line is useful during bring-up. If logs get noisy in large crawls, consider downgrading to debug.

python/src/server/services/credential_service.py (3)

242-263: Great: non-fatal provider cache invalidation on set; deduplicate key lists to avoid drift.

The duplicated provider_related_keys set appears again in delete_credential. Centralize it to a module-level constant to keep both sites in sync.

Apply this refactor:

@@
-from .llm_provider_service import invalidate_provider_cache
+from .llm_provider_service import invalidate_provider_cache
@@
-                provider_related_keys = {
-                    "LLM_PROVIDER",
-                    "MODEL_CHOICE",
-                    "EMBEDDING_MODEL",
-                    "OPENAI_API_KEY",
-                    "OPENAI_BASE_URL",
-                    "OPENAI_API_BASE",
-                    "GOOGLE_API_KEY",
-                    "OLLAMA_BASE_URL",
-                }
-                if category == "rag_strategy" or key in provider_related_keys:
+                # Keep provider-related keys in one place to avoid drift
+                from typing import Final
+                PROVIDER_RELATED_KEYS: Final[set[str]] = {
+                    "LLM_PROVIDER",
+                    "MODEL_CHOICE",
+                    "EMBEDDING_MODEL",
+                    "OPENAI_API_KEY",
+                    "OPENAI_BASE_URL",
+                    "OPENAI_API_BASE",
+                    "GOOGLE_API_KEY",
+                    "OLLAMA_BASE_URL",
+                }
+                if category == "rag_strategy" or key in PROVIDER_RELATED_KEYS:
                     invalidate_provider_cache(keys_changed=[key])

You can then reuse PROVIDER_RELATED_KEYS in delete_credential below.

---

292-310: Same note: reuse the centralized PROVIDER_RELATED_KEYS set here.

Avoid maintaining two copies of the same set.

Apply this diff:

@@
-                provider_related_keys = {
-                    "LLM_PROVIDER",
-                    "MODEL_CHOICE",
-                    "EMBEDDING_MODEL",
-                    "OPENAI_API_KEY",
-                    "OPENAI_BASE_URL",
-                    "OPENAI_API_BASE",
-                    "GOOGLE_API_KEY",
-                    "OLLAMA_BASE_URL",
-                }
-                if key in provider_related_keys:
+                from typing import Final
+                PROVIDER_RELATED_KEYS: Final[set[str]] = {
+                    "LLM_PROVIDER",
+                    "MODEL_CHOICE",
+                    "EMBEDDING_MODEL",
+                    "OPENAI_API_KEY",
+                    "OPENAI_BASE_URL",
+                    "OPENAI_API_BASE",
+                    "GOOGLE_API_KEY",
+                    "OLLAMA_BASE_URL",
+                }
+                if key in PROVIDER_RELATED_KEYS:
                     invalidate_provider_cache(keys_changed=[key])

---

459-473: OpenAI base_url resolution path is solid; consider normalizing the URL.

Some gateways expect a trailing "/v1". Normalizing avoids subtle 404s when callers omit it.

Example normalization (before returning the config):

@@
                 if not base_url:
                     base_url = os.getenv("OPENAI_BASE_URL") or os.getenv("OPENAI_API_BASE")
+            # Optional: normalize base_url to include /v1 when talking to OpenAI-compatible endpoints
+            if provider == "openai" and base_url and not re.search(r'/v1/?$', base_url):
+                base_url = base_url.rstrip('/') + '/v1'

Note: you’d need import re at top. If you don’t want implicit changes, logging a warning when /v1 is missing is a lighter alternative.

python/src/server/services/crawling/code_extraction_service.py (1)

316-321: Fallback source_id derivation is fine; consider light normalization.

URLs with trailing slashes or default ports can lead to subtly different source_ids across runs. Optional normalization (lowercasing host, stripping trailing slashes) improves deduplication.

Example:

-                    if not source_id:
-                        parsed_url = urlparse(source_url)
-                        source_id = parsed_url.netloc or parsed_url.path
+                    if not source_id:
+                        parsed_url = urlparse(source_url)
+                        host_or_path = parsed_url.netloc or parsed_url.path
+                        source_id = host_or_path.rstrip('/').lower()