Skip to content

LCORE-2755: ability to dump successful response models#2130

Merged
tisnik merged 2 commits into
lightspeed-core:mainfrom
tisnik:lcore-2755-dump-successful-response-models
Jul 14, 2026
Merged

LCORE-2755: ability to dump successful response models#2130
tisnik merged 2 commits into
lightspeed-core:mainfrom
tisnik:lcore-2755-dump-successful-response-models

Conversation

@tisnik

@tisnik tisnik commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Description

LCORE-2755: ability to dump successful response models

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

  • Assisted-by: N/A
  • Generated by: N/A

Related Tickets & Documents

  • Related Issue #LCORE-2755

Summary by CodeRabbit

  • New Features

    • Expanded OpenAPI schema coverage to include successful response models across approvals, conversations, authentication, inference, RAG, providers, health, telemetry, and related APIs.
    • Added schema definitions for OpenAI Responses API, streaming responses, saved prompts, vector stores, and endpoint configuration responses.
  • Bug Fixes

    • Corrected the schema reference used for MCP tool approval filtering.
    • Updated schema validation expectations to reflect the expanded API model coverage.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The model dumper now includes successful response models in its OpenAPI schema output. The unit test updates its expected component definitions, references, and schema-name assertions accordingly.

Changes

Schema dump expansion

Layer / File(s) Summary
Successful model collection
src/utils/models_dumper.py
dump_models() imports and collects additional successful response models for OpenAPI schema dumping.
Expected schema definitions
tests/unit/utils/test_models_dumper.py
The expected schema fixture adds configuration, conversation, MCP, OpenAI, provider, RAG, streaming, trust, and vector-store response schemas, including an updated approval-filter reference.
Schema name assertions
tests/unit/utils/test_models_dumper.py
The expected schema-name tuple is updated for the expanded dumper output.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: dumping successful response models.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity ✅ Passed src/utils/models_dumper.py:24-89 only appends a fixed list of model classes; no nested loops, N+1s, API calls, or unbounded growth.
Security And Secret Handling ✅ Passed Only schema-dump/test updates; no hardcoded creds, auth changes, injections, or sensitive runtime/log output found in the modified files.
✨ 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

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

⚠️ Outside diff range comments (1)
tests/unit/utils/test_models_dumper.py (1)

11-24: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Docstring embeds the entire ~7500-line expected JSON dump as unverified documentation.

The docstring (extending from here to line 7552) reproduces the full OpenAPI schema output purely as an example — none of it is actually asserted against; only expected_schemas presence-checks run. This makes every future schema change (like this PR) require touching thousands of docstring lines that provide no test coverage and will silently drift from reality. Consider trimming the docstring to a short illustrative snippet, or moving the full example to an external fixture file that isn't manually hand-maintained inline.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/utils/test_models_dumper.py` around lines 11 - 24, Trim the
oversized docstring in test_dump_models to a brief description with a small
representative schema snippet, removing the unverified full OpenAPI dump. Keep
the actual assertions and expected_schemas checks unchanged, and do not preserve
the thousands of manually maintained example lines inline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@tests/unit/utils/test_models_dumper.py`:
- Around line 11-24: Trim the oversized docstring in test_dump_models to a brief
description with a small representative schema snippet, removing the unverified
full OpenAPI dump. Keep the actual assertions and expected_schemas checks
unchanged, and do not preserve the thousands of manually maintained example
lines inline.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ac5bf5b2-5066-47b9-b7df-3616c7d56dd0

📥 Commits

Reviewing files that changed from the base of the PR and between a9e0a01 and 3a0253e.

📒 Files selected for processing (2)
  • src/utils/models_dumper.py
  • tests/unit/utils/test_models_dumper.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
  • GitHub Check: integration_tests (3.13)
  • GitHub Check: integration_tests (3.12)
  • GitHub Check: Pylinter
  • GitHub Check: build-pr
  • GitHub Check: unit_tests (3.12)
  • GitHub Check: E2E: server mode / ci / group 2
  • GitHub Check: E2E: library mode / ci / group 2
  • GitHub Check: E2E: library mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Use absolute imports for internal modules: from authentication import get_auth_dependency
Llama Stack imports: Use from llama_stack_client import AsyncLlamaStackClient
Check constants.py for shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Use logger = get_logger(__name__) from log.py for module logging
All functions must have complete type annotations for parameters and return types, use modern syntax (str | int), and include descriptive docstrings
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 function parameters
Use async def for I/O operations and external API calls
Use standard log levels with clear purposes: debug() for diagnostic info, info() for program execution, warning() for unexpected events, error() for serious problems
All classes must have descriptive docstrings explaining purpose and use PascalCase with standard suffixes: Configuration, Error/Exception, Resolver, Interface
Abstract classes must use ABC with @abstractmethod decorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes

Files:

  • src/utils/models_dumper.py
**/*

📄 CodeRabbit inference engine (Custom checks)

**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.

Files:

  • src/utils/models_dumper.py
  • tests/unit/utils/test_models_dumper.py
tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Use pytest.mark.asyncio marker for async tests

Files:

  • tests/unit/utils/test_models_dumper.py
🧠 Learnings (2)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.

Applied to files:

  • src/utils/models_dumper.py
  • tests/unit/utils/test_models_dumper.py
📚 Learning: 2026-07-06T15:26:18.398Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 2071
File: src/models/config.py:2416-2422
Timestamp: 2026-07-06T15:26:18.398Z
Learning: In this repo’s Python code under src/**, don’t treat differences in string concatenation style as a style inconsistency when Black has effectively forced (or made clearer) use of explicit `+` string concatenation in multi-line logger/string expressions. If adjacent-literal implicit concatenation is avoided/changed specifically to accommodate Black’s formatting in these call sites, accept the `+` usage and don’t recommend converting it solely for consistency with nearby blocks that use implicit concatenation.

Applied to files:

  • src/utils/models_dumper.py
🔇 Additional comments (3)
src/utils/models_dumper.py (2)

47-86: LGTM!


4-4: 📐 Maintainability & Code Quality

Import style here matches the existing module-alias convention in this file; no change needed.

			> Likely an incorrect or invalid review comment.
tests/unit/utils/test_models_dumper.py (1)

25-146: LGTM!

Also applies to: 160-181, 236-1268, 1290-1517, 1631-1653, 1672-1897, 1946-1946, 1977-2215, 2242-2381, 2493-2725, 2874-2892, 3879-3976, 4040-4121, 4162-4261, 4324-4490, 4694-5317, 5578-5902, 5959-5979, 6014-6084, 6131-6153, 6223-6267, 6315-6432, 6495-6586, 6611-6967, 7057-7102, 7162-7403, 7453-7546, 7577-7643, 7672-7742

@tisnik
tisnik merged commit 57f9f05 into lightspeed-core:main Jul 14, 2026
42 of 44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant