Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,7 @@ options:
-d, --dump-configuration
dump actual configuration into JSON file and quit
-s, --dump-schema dump configuration schema into OpenAPI-compatible file and quit
-m, --dump-models dump schemas for all models into OpenAPI-compatible file and quit
-c, --config CONFIG_FILE
path to configuration file (default: lightspeed-stack.yaml)
--synthesized-config-output SYNTHESIZED_CONFIG_OUTPUT
Expand Down
22 changes: 21 additions & 1 deletion src/lightspeed_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from log import get_logger, setup_logging
from runners.quota_scheduler import start_quota_scheduler
from runners.uvicorn import start_uvicorn
from utils import schema_dumper
from utils import models_dumper, schema_dumper

setup_logging()
logger = get_logger(__name__)
Expand Down Expand Up @@ -61,6 +61,14 @@ def create_argument_parser() -> ArgumentParser:
action="store_true",
default=False,
)
parser.add_argument(
"-m",
"--dump-models",
dest="dump_models",
help="dump schemas for all models into OpenAPI-compatible file and quit",
action="store_true",
default=False,
)
parser.add_argument(
"-c",
"--config",
Expand Down Expand Up @@ -105,6 +113,7 @@ def create_argument_parser() -> ArgumentParser:
return parser


# pylint: disable=too-many-branches, too-many-statements
def main() -> None:
"""Entry point to the web service.

Expand Down Expand Up @@ -193,6 +202,17 @@ def main() -> None:
raise SystemExit(1) from e
return

# -m or --dump-models CLI flags are used to dump schema for all models
# into a JSON file that is compatible with OpenAPI schema specification
if args.dump_models:
try:
models_dumper.dump_models("models.json")
logger.info("Schema for all models dumped to models.json")
except Exception as e:
logger.error("Failed to dump schema for models: %s", e)
raise SystemExit(1) from e
return

# Store config path in env so each uvicorn worker can load it
# (step is needed because process context isn't shared).
os.environ[constants.CONFIG_PATH_ENV_VAR] = args.config_file
Expand Down
51 changes: 51 additions & 0 deletions src/utils/models_dumper.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
"""Function to dump the schema of all data models into OpenAPI-compatible format."""

import json

from pydantic.json_schema import models_json_schema

import models.compaction as models_compaction
from utils.json_schema_updater import recursive_update


def dump_models(filename: str) -> None:
"""Dump the schema of all models into OpenAPI-compatible JSON file.

Parameters:
----------
- filename: str - name of file to export the schema to

Returns:
-------
- None

Raises:
------
IOError: If the file cannot be written.
"""
with open(filename, "w", encoding="utf-8") as fout:
# retrieve the schema
_, schemas = models_json_schema(
[
(model, "validation")
for model in [models_compaction.ConversationSummary]
],
ref_template="#/components/schemas/{model}",
)

# fix the schema
schemas = recursive_update(schemas)

# add all required metadata
openapi_schema = {
"openapi": "3.0.0",
"info": {
"title": "Lightspeed Core Stack",
"version": "0.3.0",
},
"components": {
"schemas": schemas.get("$defs", {}),
},
"paths": {},
}
json.dump(openapi_schema, fout, indent=4)
Comment on lines +26 to +51

@coderabbitai coderabbitai Bot Jul 9, 2026

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.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Extract shared helper to eliminate duplication with schema_dumper.py.

dump_models is nearly identical to dump_schema in src/utils/schema_dumper.py (lines 11-47). The only differences are the model list and the docstring. This is a DRY violation — both functions generate an OpenAPI wrapper around models_json_schema + recursive_update with the same metadata.

Consider extracting a shared helper that accepts a list of models and a filename, then having both dump_schema and dump_models call it.

♻️ Proposed shared helper
+def _dump_openapi_schema(models: list, filename: str) -> None:
+    """Write an OpenAPI-compatible JSON schema for the given models to a file.
+
+    Parameters:
+    ----------
+        - models: list - Pydantic model classes to include in the schema
+        - filename: str - name of file to export the schema to
+
+    Raises:
+    ------
+        IOError: If the file cannot be written.
+    """
+    with open(filename, "w", encoding="utf-8") as fout:
+        _, schemas = models_json_schema(
+            [(model, "validation") for model in models],
+            ref_template="`#/components/schemas/`{model}",
+        )
+        schemas = recursive_update(schemas)
+        openapi_schema = {
+            "openapi": "3.0.0",
+            "info": {
+                "title": "Lightspeed Core Stack",
+                "version": "0.3.0",
+            },
+            "components": {
+                "schemas": schemas.get("$defs", {}),
+            },
+            "paths": {},
+        }
+        json.dump(openapi_schema, fout, indent=4)
+
 def dump_models(filename: str) -> None:
     """Dump the schema of all models into OpenAPI-compatible JSON file.

     Parameters:
     ----------
         - filename: str - name of file to export the schema to

     Returns:
     -------
         - None

     Raises:
     ------
         IOError: If the file cannot be written.
     """
-    with open(filename, "w", encoding="utf-8") as fout:
-        # retrieve the schema
-        _, schemas = models_json_schema(
-            [(model, "validation") for model in [
-                models_compaction.ConversationSummary
-                ]],
-            ref_template="`#/components/schemas/`{model}",
-        )
-
-        # fix the schema
-        schemas = recursive_update(schemas)
-
-        # add all required metadata
-        openapi_schema = {
-            "openapi": "3.0.0",
-            "info": {
-                "title": "Lightspeed Core Stack",
-                "version": "0.3.0",
-            },
-            "components": {
-                "schemas": schemas.get("$defs", {}),
-            },
-            "paths": {},
-        }
-        json.dump(openapi_schema, fout, indent=4)
+    _dump_openapi_schema([models_compaction.ConversationSummary], filename)
🤖 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 `@src/utils/models_dumper.py` around lines 26 - 50, `dump_models` duplicates
the OpenAPI schema-writing flow already implemented in `schema_dumper.py`’s
`dump_schema`. Extract the shared logic that builds the schema via
`models_json_schema`, applies `recursive_update`, and writes the OpenAPI wrapper
into a common helper, then have both `dump_models` and `dump_schema` call it
with their respective model lists and output filename. Use the existing
`dump_models` and `dump_schema` function names to locate the call sites and keep
only the model-selection/docstring differences in each wrapper.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@coderabbitai create issue

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.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="lightspeed-core/lightspeed-stack"

body=$(cat <<'EOF'
## Summary

`dump_models` in `src/utils/models_dumper.py` duplicates the OpenAPI schema-writing logic already implemented in `dump_schema` in `src/utils/schema_dumper.py`. Both functions:

- Build a schema via `pydantic.json_schema.models_json_schema` with the same `ref_template`.
- Apply `recursive_update` post-processing.
- Wrap the result in an identical OpenAPI 3.0 structure (`openapi`, `info`, `components.schemas`, `paths`).
- Write the result to a file with the same `json.dump(..., indent=4)` call.

The only real differences are the list of models passed in and the docstring.

## Rationale

This is a DRY violation that increases maintenance burden — any future change to the OpenAPI wrapping logic (e.g., metadata, formatting, error handling) needs to be duplicated in two places.

## Affected areas

- `src/utils/models_dumper.py` (`dump_models`)
- `src/utils/schema_dumper.py` (`dump_schema`)

## Suggested change

Extract a shared private helper, e.g. `_dump_openapi_schema(models: list, filename: str) -> None`, that accepts the list of Pydantic models and the output filename, performs the schema generation/post-processing/wrapping/write, and have both `dump_models` and `dump_schema` call it with their respective model lists.

## Acceptance criteria

- [ ] A shared helper function is introduced and used by both `dump_models` and `dump_schema`.
- [ ] No behavioral change to the generated OpenAPI JSON output.
- [ ] Existing unit tests (e.g. `tests/unit/utils/test_models_dumper.py`) continue to pass.

## References

- PR: https://github.com/lightspeed-core/lightspeed-stack/pull/2096
- Review comment: https://github.com/lightspeed-core/lightspeed-stack/pull/2096#discussion_r3550097380
- Requested by: `@tisnik`
EOF
)

gh issue create \
  --repo "$repo" \
  --title "Extract shared helper to eliminate duplication between dump_models and dump_schema" \
  --body "$body" \
  --assignee "tisnik"

Length of output: 2211


@tisnik I've created the follow-up issue: #2099, assigned to you. It captures the DRY duplication between dump_models and dump_schema along with the suggested shared-helper refactor.

(=^..^=)

92 changes: 92 additions & 0 deletions tests/unit/utils/test_models_dumper.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
"""Unit tests for utils/models_dumper module."""

from json import load
from pathlib import Path

from utils.models_dumper import dump_models


def test_dump_models(tmpdir: Path) -> None:

@coderabbitai coderabbitai Bot Jul 9, 2026

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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use tmp_path instead of tmpdir for correct Path type.

The tmpdir fixture returns a py.path.local object, not pathlib.Path, so the tmpdir: Path annotation is incorrect. The tmp_path fixture returns a pathlib.Path and is the recommended fixture in modern pytest.

💚 Proposed fix
-def test_dump_models(tmpdir: Path) -> None:
+def test_dump_models(tmp_path: Path) -> None:
     """Test that models can be dump into a JSON file.

     An example of schema dump:
     {
     "openapi": "3.0.0",
     "info": {
         "title": "Lightspeed Core Stack",
         "version": "0.3.0"
     },
     "components": {
         "schemas": {
             "ConversationSummary": {
                 "description": "A single compaction-produced summary chunk.\n\nAttributes:\n",
                 "properties": {
                     "summary_text": {
                         "description": "Natural-language summary produced by the ...",
                         "title": "Summary text",
                         "type": "string"
                     },
                     "summarized_through_turn": {
                         "description": "Running total of conversation items consumed by this ...",
                         "minimum": 0,
                         "title": "Summarized through turn",
                         "type": "integer"
                     },
                     "token_count": {
                         "description": "Number of tokens in summary_text.",
                         "minimum": 0,
                         "title": "Token count",
                         "type": "integer"
                     },
                     "created_at": {
                         "description": "ISO 8601 timestamp recording when this summary ...",
                         "title": "Created at",
                         "type": "string"
                     },
                     "model_used": {
                         "description": "Fully-qualified model identifier used for the ...",
                         "title": "Model used",
                         "type": "string"
                     }
                 },
                 "required": [
                     "summary_text",
                     "summarized_through_turn",
                     "token_count",
                     "created_at",
                     "model_used"
                 ],
                 "title": "ConversationSummary",
                 "type": "object"
             }
         }
     },
     "paths": {}
     }
     """
-    filename = tmpdir / "foo.json"
+    filename = tmp_path / "foo.json"
     dump_models(str(filename))

     with open(filename, "r", encoding="utf-8") as fin:

Also applies to: 67-70

🤖 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` at line 9, The test helper signature
uses the wrong pytest fixture type: `test_dump_models` is annotated as `Path`
but receives `tmpdir`, which is a `py.path.local` object. Update
`test_dump_models` and the other affected test cases referenced by this comment
to use `tmp_path` instead, so the fixture type matches the `pathlib.Path`
annotation and the code uses the modern pytest API consistently.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@coderabbitai create issue

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.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="lightspeed-core/lightspeed-stack"

gh issue create \
  --repo "$repo" \
  --title "Use tmp_path instead of tmpdir fixture in test_models_dumper.py" \
  --assignee "tisnik" \
  --body "## Summary
In \`tests/unit/utils/test_models_dumper.py\`, the test \`test_dump_models\` (and the related fixture usage around lines 67-70) annotates the pytest fixture parameter as \`Path\` but receives it via the \`tmpdir\` fixture:

\`\`\`python
def test_dump_models(tmpdir: Path) -> None:
    ...
    filename = tmpdir / \"foo.json\"
\`\`\`

## Rationale
The \`tmpdir\` fixture returns a \`py.path.local\` object, not a \`pathlib.Path\`, so the \`tmpdir: Path\` type annotation is incorrect and misleading. Modern pytest recommends using the \`tmp_path\` fixture, which returns an actual \`pathlib.Path\` object, keeping the type annotation accurate and the code aligned with current pytest best practices.

## Affected areas
- \`tests/unit/utils/test_models_dumper.py\` (function \`test_dump_models\`, also applies to lines 67-70)

## Suggested change
Replace the \`tmpdir\` fixture parameter with \`tmp_path\`, and update usages accordingly:

\`\`\`python
def test_dump_models(tmp_path: Path) -> None:
    ...
    filename = tmp_path / \"foo.json\"
\`\`\`

## Acceptance criteria
- [ ] \`test_dump_models\` uses the \`tmp_path\` fixture instead of \`tmpdir\`.
- [ ] Type annotation \`Path\` matches the actual fixture return type.
- [ ] All usages within the test (including lines 67-70) are updated consistently.

## References
- PR: https://github.com/lightspeed-core/lightspeed-stack/pull/2096
- Review comment: https://github.com/lightspeed-core/lightspeed-stack/pull/2096#discussion_r3550097389
- Requested by: `@tisnik`"

Length of output: 1953


@tisnik I've created the follow-up issue: #2100

It captures the tmpdirtmp_path fixture fix for tests/unit/utils/test_models_dumper.py::test_dump_models, and has been assigned to you.

"""Test that models can be dump into a JSON file.

An example of schema dump:
{
"openapi": "3.0.0",
"info": {
"title": "Lightspeed Core Stack",
"version": "0.3.0"
},
"components": {
"schemas": {
"ConversationSummary": {
"description": "A single compaction-produced summary chunk.\n\nAttributes:\n",
"properties": {
"summary_text": {
"description": "Natural-language summary produced by the ...",
"title": "Summary text",
"type": "string"
},
"summarized_through_turn": {
"description": "Running total of conversation items consumed by this ...",
"minimum": 0,
"title": "Summarized through turn",
"type": "integer"
},
"token_count": {
"description": "Number of tokens in summary_text.",
"minimum": 0,
"title": "Token count",
"type": "integer"
},
"created_at": {
"description": "ISO 8601 timestamp recording when this summary ...",
"title": "Created at",
"type": "string"
},
"model_used": {
"description": "Fully-qualified model identifier used for the ...",
"title": "Model used",
"type": "string"
}
},
"required": [
"summary_text",
"summarized_through_turn",
"token_count",
"created_at",
"model_used"
],
"title": "ConversationSummary",
"type": "object"
}
}
},
"paths": {}
}
"""
filename = tmpdir / "foo.json"
dump_models(str(filename))

with open(filename, "r", encoding="utf-8") as fin:
# schema should be stored in JSON format
content = load(fin)
assert content is not None

# top-level keys test
keys = ("openapi", "info", "components", "paths")
for key in keys:
assert key in content

# components should be top-level node
components = content["components"]
assert components is not None

# schemas should be a node stored inside components node
assert "schemas" in components
schemas = components["schemas"]
assert schemas is not None

# list of schemas expected in a dump
expected_schemas = ("ConversationSummary",)
for expected_schema in expected_schemas:
assert expected_schema in schemas
Loading