LCORE-2337: Migration tool — dumb-mode lift-and-shift#2029
LCORE-2337: Migration tool — dumb-mode lift-and-shift#2029max-svistunov wants to merge 2 commits into
Conversation
Add migrate_config_dumb(), the lift-and-shift migration that converts a legacy two-file config (run.yaml + lightspeed-stack.yaml) into a unified single file. The operator's lightspeed-stack.yaml is kept verbatim except for its llama_stack section, where library_client_config_path is dropped and replaced by a config block that lifts the entire run.yaml body into native_override with baseline: empty. Synthesizing the result then starts from an empty baseline and deep-merges only the lifted run.yaml, so it reproduces the original run.yaml exactly (Decision T7) - a lossless round-trip - without trying to factor anything into high-level sections (that "smart" mode is deferred future work). All other lightspeed-stack.yaml content (name, service, byok_rag, ...) is preserved, so existing enrichment keeps working in unified mode as before. Add unit tests for the migrated structure and the migrate -> synthesize round-trip (synthesized dict equals the original run.yaml). The synthesizer module crossed pylint's 1000-line module limit, so disable too-many-lines at module scope, matching the precedent in src/models/config.py.
Expose the migration on the lightspeed-stack CLI: --migrate-config plus
--run-yaml and --migrate-output. main() dispatches to migrate_config_dumb and
exits early, before load_configuration, because the input -c file still uses
the legacy library_client_config_path shape and is read raw rather than
validated against the current schema. Missing --run-yaml/--migrate-output exits
with status 1, and the help text advises replacing literal secrets with
${env.VAR} references.
Add CLI tests: flag parsing, the main() happy path (migrates the legacy pair
and returns without starting the service), and the missing-flag exit.
WalkthroughAdds a ChangesLegacy Config Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/llama_stack_configuration.py`:
- Around line 965-966: The migrated output written in the configuration export
path currently uses the default file mode, which can leave `run.yaml` contents
overly permissive; update the write logic around `output_path`/`yaml.dump` to
explicitly set restrictive permissions consistent with the synthesized-config
path (mode 0600) so secrets in the generated file are protected. Use the
existing output-writing flow in `llama_stack_configuration.py` as the place to
apply the permission change, keeping the behavior aligned with the other config
export path.
- Around line 946-957: Handle the empty/comment-only lightspeed-stack.yaml case
in the configuration load path by checking the result of yaml.safe_load before
using lcs_config; in the llama_stack configuration block, treat None as an
invalid/empty config and fail with a clear, documented error instead of letting
dict access or assignment on lcs_config raise AttributeError/TypeError. Keep the
fix localized around the existing lcs_config, llama_stack, and
lightspeed_yaml_path handling so the behavior stays consistent for valid YAML
inputs.
🪄 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: da1a667a-3420-4de6-ac1f-28d442ae6c55
📒 Files selected for processing (4)
src/lightspeed_stack.pysrc/llama_stack_configuration.pytests/unit/test_lightspeed_stack.pytests/unit/test_llama_stack_synthesize.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: spectral
- GitHub Check: mypy
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Use absolute imports for internal modules:from authentication import get_auth_dependency
Llama Stack imports: Usefrom llama_stack_client import AsyncLlamaStackClient
Checkconstants.pyfor shared constants before defining new ones
All modules must start with descriptive docstrings explaining purpose
Uselogger = get_logger(__name__)fromlog.pyfor 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
Useasync deffor 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@abstractmethoddecorators
Follow Google Python docstring conventions with required sections: Parameters, Returns, Raises, and Attributes for classes
Files:
src/lightspeed_stack.pysrc/llama_stack_configuration.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Usepytest.mark.asynciomarker for async tests
Files:
tests/unit/test_llama_stack_synthesize.pytests/unit/test_lightspeed_stack.py
🧠 Learnings (1)
📚 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/lightspeed_stack.pysrc/llama_stack_configuration.pytests/unit/test_llama_stack_synthesize.pytests/unit/test_lightspeed_stack.py
🪛 ast-grep (0.44.0)
src/llama_stack_configuration.py
[warning] 943-943: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(run_yaml_path, "r", encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 945-945: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(lightspeed_yaml_path, "r", encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 964-964: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(output_path, "w", encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🔇 Additional comments (3)
src/lightspeed_stack.py (1)
79-103: LGTM!Also applies to: 138-154
tests/unit/test_lightspeed_stack.py (1)
24-99: LGTM!tests/unit/test_llama_stack_synthesize.py (1)
381-447: LGTM!
| with open(lightspeed_yaml_path, "r", encoding="utf-8") as file: | ||
| lcs_config = yaml.safe_load(file) | ||
|
|
||
| # Preserve the whole lightspeed-stack.yaml; only rewrite the llama_stack | ||
| # section: drop the legacy path, add the unified config block. | ||
| llama_stack = dict(lcs_config.get("llama_stack") or {}) | ||
| llama_stack.pop("library_client_config_path", None) | ||
| llama_stack["config"] = { | ||
| "baseline": "empty", | ||
| "native_override": run_yaml, | ||
| } | ||
| lcs_config["llama_stack"] = llama_stack |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Empty lightspeed-stack.yaml will raise an opaque AttributeError/TypeError.
yaml.safe_load returns None for an empty or comment-only file. Line 951 (lcs_config.get(...)) then raises AttributeError, and line 957 (lcs_config["llama_stack"] = ...) would raise TypeError — neither is among the documented OSError/YAMLError. Guard for the None case to fail clearly.
🛡️ Proposed guard
with open(lightspeed_yaml_path, "r", encoding="utf-8") as file:
lcs_config = yaml.safe_load(file)
+ if not isinstance(lcs_config, dict):
+ raise ValueError(
+ f"{lightspeed_yaml_path} did not parse to a mapping; cannot migrate."
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| with open(lightspeed_yaml_path, "r", encoding="utf-8") as file: | |
| lcs_config = yaml.safe_load(file) | |
| # Preserve the whole lightspeed-stack.yaml; only rewrite the llama_stack | |
| # section: drop the legacy path, add the unified config block. | |
| llama_stack = dict(lcs_config.get("llama_stack") or {}) | |
| llama_stack.pop("library_client_config_path", None) | |
| llama_stack["config"] = { | |
| "baseline": "empty", | |
| "native_override": run_yaml, | |
| } | |
| lcs_config["llama_stack"] = llama_stack | |
| with open(lightspeed_yaml_path, "r", encoding="utf-8") as file: | |
| lcs_config = yaml.safe_load(file) | |
| if not isinstance(lcs_config, dict): | |
| raise ValueError( | |
| f"{lightspeed_yaml_path} did not parse to a mapping; cannot migrate." | |
| ) | |
| # Preserve the whole lightspeed-stack.yaml; only rewrite the llama_stack | |
| # section: drop the legacy path, add the unified config block. | |
| llama_stack = dict(lcs_config.get("llama_stack") or {}) | |
| llama_stack.pop("library_client_config_path", None) | |
| llama_stack["config"] = { | |
| "baseline": "empty", | |
| "native_override": run_yaml, | |
| } | |
| lcs_config["llama_stack"] = llama_stack |
🤖 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/llama_stack_configuration.py` around lines 946 - 957, Handle the
empty/comment-only lightspeed-stack.yaml case in the configuration load path by
checking the result of yaml.safe_load before using lcs_config; in the
llama_stack configuration block, treat None as an invalid/empty config and fail
with a clear, documented error instead of letting dict access or assignment on
lcs_config raise AttributeError/TypeError. Keep the fix localized around the
existing lcs_config, llama_stack, and lightspeed_yaml_path handling so the
behavior stays consistent for valid YAML inputs.
| with open(output_path, "w", encoding="utf-8") as file: | ||
| yaml.dump(lcs_config, file, Dumper=YamlDumper, default_flow_style=False) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Consider restricting permissions on the migrated output file.
The lifted run.yaml may contain literal secrets, and the unified output is written with the default umask. The synthesized-config path in this same codebase is written with mode 0600; applying the same here would keep parity. (The --help guidance to use ${env.VAR} mitigates but doesn't enforce this.)
🤖 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/llama_stack_configuration.py` around lines 965 - 966, The migrated output
written in the configuration export path currently uses the default file mode,
which can leave `run.yaml` contents overly permissive; update the write logic
around `output_path`/`yaml.dump` to explicitly set restrictive permissions
consistent with the synthesized-config path (mode 0600) so secrets in the
generated file are protected. Use the existing output-writing flow in
`llama_stack_configuration.py` as the place to apply the permission change,
keeping the behavior aligned with the other config export path.
1 participant
Description
Implements LCORE-2337 — the dumb-mode migration tool, sibling of the merged
LCORE-2336 (unified
lightspeed-stack.yamlschema + synthesizer).Adds
--migrate-config, a lift-and-shift that converts a legacy two-file config(
run.yaml+lightspeed-stack.yaml) into a unified single file: it dropslibrary_client_config_pathand lifts the entirerun.yamlbody underllama_stack.config.native_overridewithbaseline: empty, preserving allother
lightspeed-stack.yamlcontent. Synthesizing the result reproduces theoriginal
run.yamlexactly (lossless round-trip), so existing enrichment keepsworking unchanged. No attempt is made to factor the run.yaml into high-level
sections — that "smart" mode is deferred future work.
migrate_config_dumb()insrc/llama_stack_configuration.py.--migrate-config,--run-yaml,--migrate-outputflags insrc/lightspeed_stack.py, dispatched inmain()beforeload_configuration(the
-cfile still uses the legacy shape and is read raw). Missing--run-yaml/--migrate-outputexits with status 1;--helpdocuments theflags and advises replacing literal secrets with
${env.VAR}references.Type of change
Tools used to create PR
Related Tickets & Documents
Checklist before requesting a review
Testing
Manual verification (migrate a legacy config, then boot it)
Migrate the repo's CI run.yaml + a legacy lightspeed-stack.yaml:
The output drops
library_client_config_pathand carries the whole run.yamlunder
llama_stack.config.native_overridewithbaseline: empty.Boot library mode with the migrated file and query it:
Lossless round-trip — synthesizing the migrated config reproduces the
original run.yaml (dict-equal):
Tests specific to this change
Covers
migrate_config_dumbstructure, the migrate→synthesize round-trip, flagparsing, the
main()dispatch, and the missing-flag exit.Full unit suite
uv run make verifypasses for all files changed in this PR (black, pylint10.00/10, ruff, pyright, pydocstyle). NOTE:
make verify'scheck-types-testsis currently red on two pre-existing upstream test-annotation issues
(
tests/unit/conftest.py,tests/integration/container_lifecycle/test_container_lifecycle.py),neither touched by this PR; CI's mypy scope passes.
Summary by CodeRabbit
New Features
Bug Fixes