fix to support istio service mesh by removing token

[vizsatiz]

Summary by CodeRabbit

• Bug Fixes

  • Improved parameter handling with stricter type validation for API configuration.

• Refactor

  • Streamlined authentication logic for improved robustness and flexibility.
  • Enhanced streaming functionality to support additional configuration options.
  • Optimized credential handling for better compatibility across different authentication scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This PR refactors LLM factory and authentication handling by casting configuration parameters to strings, simplifying RootFlo authentication validation to require only base_url, making api_token optional with fallback handling, and expanding the stream method signature with kwargs support.

Changes

Cohort / File(s)	Summary
Factory Parameter Handling flo_ai/flo_ai/helpers/llm_factory.py	Cast base_url to string for VertexAI, OpenAI vLLM, and RootFlo creation paths; cast api_key to string in OpenAI vLLM path. Simplified RootFlo authentication to check only base_url presence with strict error handling, removing multi-branch JWT validation logic.
Base LLM Interface flo_ai/flo_ai/llm/base_llm.py	Extended stream method signature to accept **kwargs: Any, providing flexibility for additional parameters while preserving existing required parameters and return type.
RootFlo LLM Auth Refactor flo_ai/flo_ai/llm/rootflo_llm.py	Made api_token parameter optional (str | None, default None) in _fetch_llm_config_async; removed JWT credential enforcement in __init__; adjusted Authorization header construction to only include when api_token is provided; use 'no_token' fallback when instantiating provider SDK wrappers if token is absent.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Authentication logic simplification in llm_factory.py: Verify the removal of multi-branch JWT validation doesn't inadvertently bypass critical checks
• Optional api_token handling in rootflo_llm.py: Ensure 'no_token' fallback doesn't break downstream provider SDK integration
• Parameter casting changes: Confirm all casting operations handle edge cases (e.g., None values, type coercion)

Poem

🐰 Strings we cast and tokens flow,
Auth rules simplified below,
Kwargs added to the stream,
Flexible LLM dreams,
Cleaner paths for requests to go! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: modifying token/authentication handling to support Istio service mesh deployments.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch istiofix

---

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

flo_ai/flo_ai/helpers/llm_factory.py (2)

141-146: Avoid converting None VertexAI base_url into the string "None"

For VertexAI, base_url is optional. If it’s omitted, base_url will be None, and str(base_url) will turn that into the literal "None", which is likely not a valid URL and may regress existing configurations that relied on the default behavior when no base_url is provided.

Consider guarding the cast:

-        return VertexAI(
-            model=model_name,
-            project=project,
-            location=location,
-            base_url=str(base_url),
-        )
+        return VertexAI(
+            model=model_name,
+            project=project,
+            location=location,
+            base_url=str(base_url) if base_url is not None else None,
+        )

---

167-179: Pass None to OpenAIVLLM when api_key is not provided instead of stringifying it

When api_key is not provided, str(None) converts it to the string "None", which the AsyncOpenAI client treats as an actual API key rather than a missing value. This prevents the client from falling back to the OPENAI_API_KEY environment variable and instead attempts authentication with the literal string "None", causing authentication failures.

Pass None directly when the key is absent:

         return OpenAIVLLM(
             model=model_name,
             base_url=str(base_url),
-            api_key=str(api_key),
+            api_key=str(api_key) if api_key is not None else None,
             temperature=temperature,
         )

flo_ai/flo_ai/llm/base_llm.py (1)

28-35: OllamaLLM.stream() missing **kwargs—add it to match abstract signature

The abstract stream method now accepts **kwargs, but OllamaLLM.stream() does not include this parameter. Its signature only has messages and functions, which means any polymorphic call to a BaseLLM instance with extra kwargs will raise TypeError if it's an OllamaLLM.

Update OllamaLLM.stream() in flo_ai/flo_ai/llm/ollama_llm.py (line 73) to accept **kwargs: Any to match the abstract method signature. All other subclasses (OpenAI, Gemini, Anthropic, RootFloLLM) already have this parameter.

flo_ai/flo_ai/llm/rootflo_llm.py (1)

175-196: Add validation for JWT required fields before token generation

JWT generation at lines 179–191 assumes _issuer and _audience are present, but both are Optional[str] with None defaults (lines 37–38). The payload is built without validation, resulting in 'iss': None and 'aud': None when these fields are not provided. Add a check before generating JWT:

-            elif self._app_key and self._app_secret:
+            elif self._app_key and self._app_secret and self._issuer and self._audience:

Replace 'no_token' sentinel with None for missing API keys

Lines 225, 234, 243, and 254 use api_token or 'no_token'. The 'no_token' string sends a literal Authorization: Bearer no_token header, which differs from the SDK's expected behavior when api_key is None. With None, OpenAI and Anthropic SDKs properly fall back to environment variables or internal handling. To preserve correct SDK semantics:

-                    api_key=api_token or 'no_token',
+                    api_key=api_token,

Apply this change for OpenAI, Anthropic, Gemini, and OpenAIVLLM instantiations. (Note: This may require adjusting your Istio mesh configuration if 'no_token' was relied upon as a workaround for request routing.)

🧹 Nitpick comments (1)

flo_ai/flo_ai/llm/rootflo_llm.py (1)

98-105: Optional api_token and conditional Authorization header align with mesh-based auth; update docstring

Making api_token optional and only adding the Authorization header when it’s present is exactly what you want for Istio/service‑mesh scenarios where auth is injected.

Minor nit: the docstring for api_token still reads as if it’s always supplied (“The JWT token for authorization”). Consider clarifying that it’s optional and that when omitted, no Authorization header is sent.

Also applies to: 120-128

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31c3e68 and cf99b40.

📒 Files selected for processing (3)

• flo_ai/flo_ai/helpers/llm_factory.py (3 hunks)
• flo_ai/flo_ai/llm/base_llm.py (1 hunks)
• flo_ai/flo_ai/llm/rootflo_llm.py (7 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

flo_ai/flo_ai/helpers/llm_factory.py (1)

flo_ai/flo_ai/llm/rootflo_llm.py (1)

• RootFloLLM (25-307)

🔇 Additional comments (2)

flo_ai/flo_ai/helpers/llm_factory.py (1)

192-219: RootFlo base_url validation looks good; string cast is safe here

The new RootFlo block now only requires base_url (from kwargs, model_config, or ROOTFLO_BASE_URL), which aligns with the intent to support Istio/mesh‑based auth. The error message is clear and centralizes validation.

Because of the preceding if not base_url: raise ValueError, base_url=str(base_url) in the RootFloLLM ctor is safe and should not suffer from the "None" issue mentioned for VertexAI.

flo_ai/flo_ai/llm/rootflo_llm.py (1)

278-284: RootFloLLM.stream now cleanly matches the extended interface

The stream method here now accepts **kwargs and passes them through to the underlying _llm.stream, which keeps RootFloLLM in sync with the updated BaseLLM.stream and allows extra streaming options without further changes.