Parsing: fix system config detection#881
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughReplaces substring-based TOML heuristic in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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/cloudai/cli/handlers.py`:
- Around line 554-555: The current check if "\nscheduler =" in content can miss
files where a "scheduler =" key appears at the start or is indented; replace
that substring test with a line-anchored check that iterates
content.splitlines() and treats a line as a match when
line.lstrip().startswith("scheduler =") (so the branch that calls
files["system"].append(toml_file) triggers for leading-byte or indented keys).
Update the same matcher logic in tests/test_toml_files.py so production and
tests use identical line-anchored detection.
🪄 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 Plus
Run ID: b23ab1c0-e001-478c-a971-4b3e52e4d563
📒 Files selected for processing (1)
src/cloudai/cli/handlers.py
Contributor
There was a problem hiding this comment.
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/cloudai/cli/handlers.py`:
- Around line 554-561: When toml.loads(content) raises TomlDecodeError the file
is currently skipped; change the except block to record the path into the files
mapping (e.g. files["unknown"] or a new files["invalid_toml"]) instead of
continuing, so malformed TOML files are surfaced by handle_verify_all_configs;
update any downstream logic in handle_verify_all_configs that aggregates counts
to include this new category (or unknown) so these errors are reported as
failures. Ensure you reference the toml.loads call and the files dict
(files["unknown"] or files["invalid_toml"]) when making the change.
🪄 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 Plus
Run ID: bc49d04a-5b27-4aec-8d7f-a85052848a0b
📒 Files selected for processing (1)
src/cloudai/cli/handlers.py
Contributor
There was a problem hiding this comment.
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 `@tests/test_toml_files.py`:
- Around line 29-41: get_all_systems currently classifies files as system TOMLs
purely by presence of a top-level "scheduler" key, but runtime code in
src/cloudai/cli/handlers.py filters out hook-root TOMLs before scheduler
detection; update get_all_systems (which iterates TOML_FILES) to first apply the
same hook-root detection used in handlers.py (or import/reuse that predicate if
exposed) and skip those files, then perform the existing "scheduler" key check
so test classification matches runtime behavior.
🪄 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 Plus
Run ID: 93a0a579-944e-4e6a-aa1c-e002116bbab6
📒 Files selected for processing (1)
tests/test_toml_files.py
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/test_toml_files.py (1)
29-41: 🛠️ Refactor suggestion | 🟠 MajorKeep system test-file selection sourced from runtime classifier.
Line 29 re-implements a subset of system classification logic, and Line 56 depends on it. This can drift from
src/cloudai/cli/handlers.py::load_tomls_by_type(...)again as classification rules evolve.Proposed change
+from cloudai.cli.handlers import load_tomls_by_type + def get_all_systems() -> list[Path]: - result = [] - - for p in TOML_FILES: - try: - toml_content = toml.loads(p.read_text()) - except toml.TomlDecodeError: - continue - - if isinstance(toml_content, dict) and "scheduler" in toml_content: - result.append(p) - - return result + return load_tomls_by_type(TOML_FILES)["system"]Also applies to: 56-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_toml_files.py` around lines 29 - 41, The test reimplements system classification in get_all_systems; instead import and call the runtime classifier used in production (load_tomls_by_type from src.cloudai.cli.handlers) so test selection stays in sync: replace get_all_systems logic with a call to load_tomls_by_type(...) (or a thin wrapper that returns the same list[Path]) and adjust any callers (including the code around line 56) to use that result, ensuring the returned type matches list[Path].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/test_toml_files.py`:
- Around line 29-41: The test reimplements system classification in
get_all_systems; instead import and call the runtime classifier used in
production (load_tomls_by_type from src.cloudai.cli.handlers) so test selection
stays in sync: replace get_all_systems logic with a call to
load_tomls_by_type(...) (or a thin wrapper that returns the same list[Path]) and
adjust any callers (including the code around line 56) to use that result,
ensuring the returned type matches list[Path].
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8087fbc5-a689-4ed0-aca5-80abe4225084
📒 Files selected for processing (1)
tests/test_toml_files.py
amaslenn
approved these changes
Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scheduler =pattern was detected in a config withsomething_scheduler =being part of cmd argsTest Plan
verify-configson CloudAIx repoAdditional Notes