RSPEED-2809: Reduce infer_endpoint cyclomatic complexity by major · Pull Request #1463 · lightspeed-core/lightspeed-stack

major · 2026-04-06T20:02:40Z

Description

Extract helpers from infer_endpoint to reduce its cyclomatic complexity from C(13) to B(7). Pure refactor, no behavior changes.

The verbose mode branching was the main contributor: the same verbose_enabled check appeared in four places (config check, inference fork, failure compensation, response building). This PR consolidates those into focused helpers:

_call_llm(): Transport-only LLM call. Returns OpenAIResponseObject without metrics side effects. Callers handle token usage extraction.
_is_verbose_enabled(): The 3-way configuration.customization is not None and allow_verbose_infer and include_metadata check.
_build_infer_response(): Builds the final response. Keyed on response object presence (not a separate boolean), so response=None means minimal, response=<object> means verbose with metadata.

retrieve_simple_response() now delegates to _call_llm() and handles its own extract_token_usage() call, preserving the existing public API.

Complexity before/after

Function	Before	After
`infer_endpoint`	C (13)	B (7)
`_call_llm`	-	A (3)
`_is_verbose_enabled`	-	A (3)
`_build_infer_response`	-	A (2)
`retrieve_simple_response`	A (3)	A (2)
File average	A (4.0)	A (3.4)

Type of change

Refactor

Tools used to create PR

Assisted-by: Claude (via OpenCode)
Generated by: N/A

Related Tickets & Documents

Related Issue: N/A

Checklist before requesting a review

I have performed a self-review of my code.
PR has passed all pre-merge test jobs.
If it is a core feature, I have added thorough tests.

Testing

uv run make format && uv run make verify - all linters pass clean
uv run pytest tests/unit/app/endpoints/test_rlsapi_v1.py tests/integration/endpoints/test_rlsapi_v1_integration.py -v - all 60 tests pass (43 unit + 17 integration)
uv run radon cc src/app/endpoints/rlsapi_v1.py -s -a - no function above B, file average A(3.4)
Verified token usage extraction is called exactly once per request path (non-verbose via retrieve_simple_response, verbose via build_turn_summary in _build_infer_response)

Summary by CodeRabbit

Refactor
- Improved internal handling of inference responses for better maintainability.
Bug Fixes / Behavior
- More consistent inclusion of verbose metadata and token-usage info (now reported even on certain failures).
- Verbose vs minimal response selection consolidated so visible outputs (text and metadata) behave more predictably.

coderabbitai · 2026-04-06T20:05:17Z

Walkthrough

Refactored infer endpoint flow: added _call_llm, _is_verbose_enabled, and _build_infer_response; retrieve_simple_response now resolves model ID, calls the LLM helper, and extracts token usage and text from the returned response object.

Changes

Cohort / File(s)	Summary
Endpoint + Helpers `src/app/endpoints/rlsapi_v1.py`	Added `_call_llm(...)` to centralize transport calls; added `_is_verbose_enabled(...)` for the dual opt‑in check; added `_build_infer_response(...)` to produce verbose vs minimal payloads. Reworked `infer_endpoint` to always call `_call_llm(...)`.
Simple Response Retrieval `src/app/endpoints/rlsapi_v1.py`	Refactored `retrieve_simple_response` to resolve `resolved_model_id` via fallback, call `_call_llm(...)`, and extract response text and token usage from the returned response object.
Error / Token Usage Handling `src/app/endpoints/rlsapi_v1.py`	Adjusted exception handling so token-usage extraction on failure runs when a response object exists regardless of verbose mode; moved token extraction logic to callers of `_call_llm(...)`.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant InferEndpoint as InferEndpoint/_handler
    participant LLMHelper as _call_llm
    participant OpenAI
    participant Builder as _build_infer_response

    Client->>InferEndpoint: POST /infer (infer_request)
    InferEndpoint->>LLMHelper: await _call_llm(infer_request, resolved_model_id)
    LLMHelper->>OpenAI: client.responses.create(...)
    OpenAI-->>LLMHelper: OpenAIResponseObject
    LLMHelper-->>InferEndpoint: OpenAIResponseObject
    InferEndpoint->>Builder: _build_infer_response(response or None, include_metadata, ...)
    Builder-->>InferEndpoint: formatted response (verbose or minimal)
    InferEndpoint-->>Client: HTTP response

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

🚥 Pre-merge checks | ✅ 3

✅ 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 reflects the main objective of the PR: reducing cyclomatic complexity in the infer_endpoint function through refactoring.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

✨ Simplify code

Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)

src/app/endpoints/rlsapi_v1.py (1)

567-568: ⚠️ Potential issue | 🟡 Minor

Overly restrictive condition for token usage extraction in error handler.

The condition verbose_enabled and response is not None means token usage won't be extracted in error cases when verbose mode is disabled. If a response was received before an exception occurred, token usage should be recorded regardless of verbose mode to maintain accurate metrics.

🔧 Proposed fix

     except _INFER_HANDLED_EXCEPTIONS as error:
-        if verbose_enabled and response is not None:
-            extract_token_usage(response.usage, model_id)  # type: ignore[arg-type]
+        if response is not None:
+            extract_token_usage(response.usage, model_id)  # type: ignore[arg-type]
         _record_inference_failure(

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/app/endpoints/rlsapi_v1.py` around lines 567 - 568, The current check `if
verbose_enabled and response is not None:` prevents extracting token usage when
an API response exists but verbose is false; change the logic so that
extract_token_usage(response.usage, model_id) is invoked whenever a response
with usage was received (i.e., if response is not None and response.usage is
present) regardless of verbose_enabled, while keeping any verbose-only logging
guarded by verbose_enabled; update the code around the error-handler block that
references verbose_enabled, response and calls extract_token_usage to first
check response (and response.usage) and then call extract_token_usage, moving it
out of the verbose-only branch.

🤖 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/app/endpoints/rlsapi_v1.py`:
- Around line 266-271: The tools parameter of _call_llm currently uses the Any
type; replace it with the concrete MCP tool type (or a module-level type alias)
to satisfy typing rules—import the MCP tool interface/TypedDict/Protocol if
available and change the signature tools: Optional[list[Any]] to tools:
Optional[list[McpToolType]] (or define McpToolType = ... at top of the module
and use that) and update any internal usages to that type to ensure correct
static typing for the _call_llm function.
- Around line 558-564: In infer_endpoint, after calling _call_llm and assigning
response (the same way retrieve_simple_response does), call
extract_token_usage(response.usage, resolved_model_id) on the successful path so
token metrics are recorded; locate the response variable returned from _call_llm
in infer_endpoint and add the extract_token_usage call before proceeding to
build response_text (mirroring retrieve_simple_response's usage extraction),
leaving the existing exception-path extract_token_usage intact.

---

Outside diff comments:
In `@src/app/endpoints/rlsapi_v1.py`:
- Around line 567-568: The current check `if verbose_enabled and response is not
None:` prevents extracting token usage when an API response exists but verbose
is false; change the logic so that extract_token_usage(response.usage, model_id)
is invoked whenever a response with usage was received (i.e., if response is not
None and response.usage is present) regardless of verbose_enabled, while keeping
any verbose-only logging guarded by verbose_enabled; update the code around the
error-handler block that references verbose_enabled, response and calls
extract_token_usage to first check response (and response.usage) and then call
extract_token_usage, moving it out of the verbose-only branch.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: be74eecd-062d-4255-a72f-c7dceecbcdfa

📥 Commits

Reviewing files that changed from the base of the PR and between 4961334 and 23cc2cc.

📒 Files selected for processing (1)

src/app/endpoints/rlsapi_v1.py

📜 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). (3)

GitHub Check: build-pr
GitHub Check: E2E: library mode / ci
GitHub Check: E2E: server mode / ci

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Use absolute imports for internal modules (e.g., from authentication import get_auth_dependency)
Use from llama_stack_client import AsyncLlamaStackClient for Llama Stack imports
Check constants.py for shared constants before defining new ones
All modules start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for module logging
Type aliases defined at module level for clarity
All functions require docstrings with brief descriptions
Use complete type annotations for function parameters and return types
Use union types with modern syntax: str | int instead of Union[str, int]
Use Optional[Type] for optional types in type annotations
Use snake_case with descriptive, action-oriented names for functions (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns: return new data structures instead of modifying parameters
Use async def for I/O operations and external API calls
Handle APIConnectionError from Llama Stack in error handling
Use logger.debug() for detailed diagnostic information
Use logger.info() for general information about program execution
Use logger.warning() for unexpected events or potential problems
Use logger.error() for serious problems that prevented function execution
All classes require descriptive docstrings explaining purpose
Use PascalCase for class names with descriptive names and standard suffixes: Configuration, Error/Exception, Resolver, Interface
Use complete type annotations for all class attributes; avoid using Any
Follow Google Python docstring conventions for all modules, classes, and functions
Include Parameters:, Returns:, Raises: sections in function docstrings as needed

Files:

src/app/endpoints/rlsapi_v1.py

src/app/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/app/**/*.py: Use from fastapi import APIRouter, HTTPException, Request, status, Depends for FastAPI dependencies
Use FastAPI HTTPException with appropriate status codes for API endpoint error handling

Files:

src/app/endpoints/rlsapi_v1.py

🔇 Additional comments (4)

src/app/endpoints/rlsapi_v1.py (4)

256-263: LGTM - Token usage extraction properly preserved.

The refactor correctly maintains token usage extraction and text extraction as caller responsibilities after delegating to _call_llm().

386-403: LGTM - Clean extraction of verbose mode check.

The three-way check is correctly encapsulated with clear documentation explaining the dual opt-in requirement.

406-456: LGTM - Response construction logic is well-structured.

The function correctly bifurcates between verbose (with build_turn_summary metadata) and minimal responses based on presence of the response object.

604-609: LGTM - Clean conditional response passing.

Using response if verbose_enabled else None is an elegant way to signal verbose mode to _build_infer_response, aligning with its documented behavior.

tisnik · 2026-04-07T08:34:27Z

Please rebase, "branching" images were introduced in different PR.

tisnik · 2026-04-07T10:38:07Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Undefined client The `_call_llm` helper references `client` without defining or importing it, causing a NameError when it attempts to call `client.responses.create(...)`. Initialize `client` (e.g., via AsyncLlamaStackClientHolder().get_client()) before use. async def _call_llm( question: str, instructions: str, tools: Optional[list[Any]] = None, model_id: Optional[str] = None, ) -> OpenAIResponseObject: """Call the LLM via the Responses API and return the full response object. This is a transport-only function: it calls the LLM and returns the raw response. Callers are responsible for token usage extraction and metrics. Args: question: The combined user input (question + context). instructions: System instructions for the LLM. tools: Optional list of MCP tool definitions for the LLM. model_id: Fully qualified model identifier in provider/model format. When omitted, the configured default model is used. Returns: The full OpenAIResponseObject from the LLM. Raises: APIConnectionError: If the Llama Stack service is unreachable. HTTPException: 503 if no default model is configured. """ client = AsyncLlamaStackClientHolder().get_client() resolved_model_id = model_id or await _get_default_model_id() logger.debug("Using model %s for rlsapi v1 inference", resolved_model_id) response = await client.responses.create( input=question, model=resolved_model_id, instructions=instructions, tools=tools or [], stream=False, store=False, ) return cast(OpenAIResponseObject, response)

Extract helpers from infer_endpoint to eliminate the verbose mode branching that inflated its complexity: - _call_llm: transport-only LLM call (no metrics side effects) - _is_verbose_enabled: the 3-way config+request check - _build_infer_response: verbose vs minimal response construction, keyed on response object presence rather than a boolean flag retrieve_simple_response now delegates to _call_llm internally and handles its own token usage extraction. The verbose failure path in infer_endpoint is preserved: if the LLM call succeeded but later processing fails, token usage is still recorded. No behavior changes, pure refactor. Signed-off-by: Major Hayden <major@redhat.com>

tisnik

LGTM

coderabbitai

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 `@src/app/endpoints/rlsapi_v1.py`:
- Around line 633-634: The code unconditionally calls
extract_token_usage(response.usage, model_id) which causes double-counting when
the verbose-success path later invokes _build_infer_response() and
build_turn_summary() that also record usage; modify the call site so
extract_token_usage is only executed when not using the verbose/summary path
(e.g., guard the call with the same verbose flag used by _build_infer_response
or skip it when build_turn_summary() will run), ensuring only one of
extract_token_usage or build_turn_summary records token usage for a given
request.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9d052c01-1b9f-43a7-b12c-ea42a9443040

📥 Commits

Reviewing files that changed from the base of the PR and between 23cc2cc and b180204.

📒 Files selected for processing (1)

src/app/endpoints/rlsapi_v1.py

📜 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). (4)

GitHub Check: build-pr
GitHub Check: E2E: library mode / ci
GitHub Check: E2E Tests for Lightspeed Evaluation job
GitHub Check: E2E: server mode / ci

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Use absolute imports for internal modules (e.g., from authentication import get_auth_dependency)
Use from llama_stack_client import AsyncLlamaStackClient for Llama Stack imports
Check constants.py for shared constants before defining new ones
All modules start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for module logging
Type aliases defined at module level for clarity
All functions require docstrings with brief descriptions
Use complete type annotations for function parameters and return types
Use union types with modern syntax: str | int instead of Union[str, int]
Use Optional[Type] for optional types in type annotations
Use snake_case with descriptive, action-oriented names for functions (get_, validate_, check_)
Avoid in-place parameter modification anti-patterns: return new data structures instead of modifying parameters
Use async def for I/O operations and external API calls
Handle APIConnectionError from Llama Stack in error handling
Use logger.debug() for detailed diagnostic information
Use logger.info() for general information about program execution
Use logger.warning() for unexpected events or potential problems
Use logger.error() for serious problems that prevented function execution
All classes require descriptive docstrings explaining purpose
Use PascalCase for class names with descriptive names and standard suffixes: Configuration, Error/Exception, Resolver, Interface
Use complete type annotations for all class attributes; avoid using Any
Follow Google Python docstring conventions for all modules, classes, and functions
Include Parameters:, Returns:, Raises: sections in function docstrings as needed

Files:

src/app/endpoints/rlsapi_v1.py

src/app/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/app/**/*.py: Use from fastapi import APIRouter, HTTPException, Request, status, Depends for FastAPI dependencies
Use FastAPI HTTPException with appropriate status codes for API endpoint error handling

Files:

src/app/endpoints/rlsapi_v1.py

🧠 Learnings (4)

📚 Learning: 2026-04-06T20:18:07.852Z

Learnt from: major
Repo: lightspeed-core/lightspeed-stack PR: 1463
File: src/app/endpoints/rlsapi_v1.py:266-271
Timestamp: 2026-04-06T20:18:07.852Z
Learning: In the lightspeed-stack codebase, within `src/app/endpoints/` inference/MCP endpoints, treat `tools: Optional[list[Any]]` in MCP tool definitions as an intentional, consistent typing pattern (used across `query`, `responses`, `streaming_query`, `rlsapi_v1`). Do not raise or suggest this as a typing issue during code review; changing it in isolation could break endpoint typing consistency across the codebase.

Applied to files:

src/app/endpoints/rlsapi_v1.py

📚 Learning: 2026-04-05T12:19:36.009Z

Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-05T12:19:36.009Z
Learning: Applies to src/**/*.py : Use complete type annotations for all class attributes; avoid using `Any`

Applied to files:

src/app/endpoints/rlsapi_v1.py

📚 Learning: 2026-02-23T14:57:07.030Z

Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1198
File: src/utils/responses.py:184-192
Timestamp: 2026-02-23T14:57:07.030Z
Learning: In the lightspeed-stack codebase, duplicate `client.models.list()` calls in model selection flows (e.g., in `src/utils/responses.py` prepare_responses_params) are acceptable as relatively cheap operations; avoiding such duplication should not introduce unnecessary complexity to the flow.

Applied to files:

src/app/endpoints/rlsapi_v1.py

📚 Learning: 2026-02-25T07:46:39.608Z

Learnt from: asimurka
Repo: lightspeed-core/lightspeed-stack PR: 1211
File: src/models/responses.py:8-16
Timestamp: 2026-02-25T07:46:39.608Z
Learning: In the lightspeed-stack codebase, src/models/requests.py uses OpenAIResponseInputTool as Tool while src/models/responses.py uses OpenAIResponseTool as Tool. This type difference is intentional - input tools and output/response tools have different schemas in llama-stack-api.

Applied to files:

src/app/endpoints/rlsapi_v1.py

🔇 Additional comments (1)

src/app/endpoints/rlsapi_v1.py (1)

267-304: Centralizing the raw LLM transport here looks good.

This helper now owns client lookup and responses.create(...), so the callers no longer depend on an outer-scope client and can stay focused on post-processing.

major changed the title ~~Reduce infer_endpoint cyclomatic complexity from C(13) to B(7)~~ Reduce infer_endpoint cyclomatic complexity Apr 6, 2026

major changed the title ~~Reduce infer_endpoint cyclomatic complexity~~ rlsapi_v1: reduce infer_endpoint cyclomatic complexity Apr 6, 2026

coderabbitai Bot reviewed Apr 6, 2026

View reviewed changes

Comment thread src/app/endpoints/rlsapi_v1.py

Comment thread src/app/endpoints/rlsapi_v1.py

major force-pushed the refactor/rlsapi-v1-reduce-complexity branch from 23cc2cc to d4ea82f Compare April 6, 2026 20:17

major changed the title ~~rlsapi_v1: reduce infer_endpoint cyclomatic complexity~~ RSPEED-2809: Reduce infer_endpoint cyclomatic complexity Apr 6, 2026

tisnik added the Review effort 1/5 label Apr 7, 2026

major force-pushed the refactor/rlsapi-v1-reduce-complexity branch from d4ea82f to b180204 Compare April 7, 2026 12:24

tisnik approved these changes Apr 7, 2026

View reviewed changes

coderabbitai Bot reviewed Apr 7, 2026

View reviewed changes

Comment thread src/app/endpoints/rlsapi_v1.py

major mentioned this pull request Apr 7, 2026

RSPEED-2817: add configuration guard to rlsapi_v1 /infer endpoint #1468

Closed

4 tasks

tisnik merged commit 8b0938e into lightspeed-core:main Apr 7, 2026
22 of 24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RSPEED-2809: Reduce infer_endpoint cyclomatic complexity#1463

RSPEED-2809: Reduce infer_endpoint cyclomatic complexity#1463
tisnik merged 1 commit into
lightspeed-core:mainfrom
major:refactor/rlsapi-v1-reduce-complexity

major commented Apr 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 6, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

tisnik commented Apr 7, 2026

Uh oh!

tisnik commented Apr 7, 2026

Uh oh!

tisnik left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

major commented Apr 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Complexity before/after

Type of change

Tools used to create PR

Related Tickets & Documents

Checklist before requesting a review

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tisnik commented Apr 7, 2026

Uh oh!

tisnik commented Apr 7, 2026

PR Reviewer Guide 🔍

Uh oh!

tisnik left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

major commented Apr 6, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 6, 2026 •

edited

Loading