Conversation
Signed-off-by: ainetx <viator@via-net.org>
…cy across multiple files Signed-off-by: ainetx <viator@via-net.org>
…erence Signed-off-by: ainetx <viator@via-net.org>
📝 WalkthroughWalkthroughThis PR systematically modernizes the studio codebase by enabling additional Pylint rules and refactoring Python idioms throughout 40+ files. Changes replace explicit ChangesCode Modernization Across Studio & Proxy
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/studio/scripts/studio/utils/document.py (1)
96-107:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore
has_priorityto a strict boolean.
priority is not None and str(priority).strip()returns a string when present, sohas_priorityis no longer consistentlybool. That changes thescan_cpt_idshit schema across definition/reference paths.💡 Proposed fix
h: Dict[str, object] = { "id": id_value, "line": idx0 + 1, "type": "definition", "checked": checked, "has_task": m.group("task") is not None, - "has_priority": priority is not None and str(priority).strip(), + "has_priority": bool(priority is not None and str(priority).strip()), } @@ h = { "id": mref.group("id"), "line": idx0 + 1, "type": "reference", "checked": checked, "has_task": mref.group("task") is not None, - "has_priority": priority is not None and str(priority).strip(), + "has_priority": bool(priority is not None and str(priority).strip()), }Also applies to: 122-132
🤖 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 `@skills/studio/scripts/studio/utils/document.py` around lines 96 - 107, The "has_priority" field in the h dict is sometimes a string because it uses "priority is not None and str(priority).strip()"; change it to a strict boolean by assigning bool(str(priority).strip()) (or simply bool(priority and str(priority).strip())) so "has_priority" is always True/False; update the same pattern wherever the other definition/reference dict is created (the other h-like dict that sets "id", "line", "type", "checked", "has_task", "has_priority") to use the same boolean conversion for the "has_priority" key.
🤖 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 `@skills/studio/scripts/studio/commands/validate.py`:
- Around line 1115-1116: The current check (if k in _HANDLED_KEYS or v is None
or not v or v == []: continue) incorrectly drops other falsy but meaningful
values like False and 0; update the condition to only skip keys in
_HANDLED_KEYS, None, and genuinely empty lists/iterables. Replace the clause
with something like: check membership in _HANDLED_KEYS first, skip when v is
None, and for empty collections check specifically (e.g., isinstance(v, list)
and len(v) == 0) or use a utility that distinguishes empty sequences from falsy
scalars so that False and 0 are preserved for rendering (keep references to
_HANDLED_KEYS, k, and v when making the change).
---
Outside diff comments:
In `@skills/studio/scripts/studio/utils/document.py`:
- Around line 96-107: The "has_priority" field in the h dict is sometimes a
string because it uses "priority is not None and str(priority).strip()"; change
it to a strict boolean by assigning bool(str(priority).strip()) (or simply
bool(priority and str(priority).strip())) so "has_priority" is always
True/False; update the same pattern wherever the other definition/reference dict
is created (the other h-like dict that sets "id", "line", "type", "checked",
"has_task", "has_priority") to use the same boolean conversion for the
"has_priority" key.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec6c3303-e81e-47ff-8529-2a8827f44efd
📒 Files selected for processing (35)
pyproject.tomlskills/studio/scripts/studio/cli.pyskills/studio/scripts/studio/commands/agents.pyskills/studio/scripts/studio/commands/doctor.pyskills/studio/scripts/studio/commands/kit.pyskills/studio/scripts/studio/commands/map/cli.pyskills/studio/scripts/studio/commands/map/cpt_edges.pyskills/studio/scripts/studio/commands/map/scan.pyskills/studio/scripts/studio/commands/migrate_from_cypilot.pyskills/studio/scripts/studio/commands/self_check.pyskills/studio/scripts/studio/commands/spec_coverage.pyskills/studio/scripts/studio/commands/update.pyskills/studio/scripts/studio/commands/validate.pyskills/studio/scripts/studio/commands/workspace_init.pyskills/studio/scripts/studio/commands/workspace_sync.pyskills/studio/scripts/studio/ralphex_discover.pyskills/studio/scripts/studio/ralphex_export.pyskills/studio/scripts/studio/utils/artifacts_meta.pyskills/studio/scripts/studio/utils/constraints.pyskills/studio/scripts/studio/utils/context.pyskills/studio/scripts/studio/utils/coverage.pyskills/studio/scripts/studio/utils/diff_engine.pyskills/studio/scripts/studio/utils/document.pyskills/studio/scripts/studio/utils/files.pyskills/studio/scripts/studio/utils/git_kit_source.pyskills/studio/scripts/studio/utils/git_utils.pyskills/studio/scripts/studio/utils/language_config.pyskills/studio/scripts/studio/utils/manifest.pyskills/studio/scripts/studio/utils/parsing.pyskills/studio/scripts/studio/utils/pdsl.pyskills/studio/scripts/studio/utils/toc.pyskills/studio/scripts/studio/utils/toml_utils.pyskills/studio/scripts/studio/utils/whatsnew.pysrc/studio_proxy/cli.pysrc/studio_proxy/update_check.py
💤 Files with no reviewable changes (1)
- skills/studio/scripts/studio/commands/doctor.py
Comment on lines
+1115
to
1116
| if k in _HANDLED_KEYS or v is None or not v or v == []: | ||
| continue |
There was a problem hiding this comment.
Avoid dropping falsy diagnostic fields in issue rendering.
Line 1115 now skips all falsy values, which can hide meaningful fields like False and 0 and breaks the “nothing is lost” intent in this formatter.
💡 Suggested fix
- if k in _HANDLED_KEYS or v is None or not v or v == []:
+ if k in _HANDLED_KEYS or v is None:
+ continue
+ if isinstance(v, str) and v == "":
+ continue
+ if isinstance(v, (list, tuple, set, dict)) and not v:
continue📝 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.
Suggested change
| if k in _HANDLED_KEYS or v is None or not v or v == []: | |
| continue | |
| if k in _HANDLED_KEYS or v is None: | |
| continue | |
| if isinstance(v, str) and v == "": | |
| continue | |
| if isinstance(v, (list, tuple, set, dict)) and not v: | |
| continue |
🤖 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 `@skills/studio/scripts/studio/commands/validate.py` around lines 1115 - 1116,
The current check (if k in _HANDLED_KEYS or v is None or not v or v == []:
continue) incorrectly drops other falsy but meaningful values like False and 0;
update the condition to only skip keys in _HANDLED_KEYS, None, and genuinely
empty lists/iterables. Replace the clause with something like: check membership
in _HANDLED_KEYS first, skip when v is None, and for empty collections check
specifically (e.g., isinstance(v, list) and len(v) == 0) or use a utility that
distinguishes empty sequences from falsy scalars so that False and 0 are
preserved for rendering (keep references to _HANDLED_KEYS, k, and v when making
the change).
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
1 participant
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.
Improve code readability and consistency across multiple files. Include a prioritized backlog of Pylint rules for future reference.
Summary by CodeRabbit
Release Notes