Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: vitali87/code-graph-rag/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe change adds constants and enum identifiers for ChangesSignature change support
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~15 minutes Change: Feature Merge Risk: ⚪ Minimal · up to No actionable issue remains in the reviewed change; it is ready to merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
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 |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| _SPEC_RE = re.compile( | ||
| r"^(?P<name>[A-Za-z_][A-Za-z0-9_]*)" | ||
| r"(?::(?P<annotation>[^=@]+))?" | ||
| r"(?:=(?P<default>[^@]+))?" |
There was a problem hiding this comment.
A quoted default such as email:str="a@b" is split at the @: the default becomes "a and b" is recorded as the source parameter. This rejects or incorrectly remaps valid email-like string defaults when changing a signature.
Artifacts
- The focused reproduction supplies a quoted default containing an at sign and asserts that it remains intact.
- The output shows the default was truncated and the remainder was treated as a source mapping, confirming the parsing failure.
Quoted default reference parser
- The reference parser demonstrates the expected intact quoted-default interpretation.
Quoted default reference output
- The reference output retains the complete default without creating a source mapping.
| "dict": frozenset({"dict"}), | ||
| "set": frozenset({"set"}), | ||
| "tuple": frozenset({"tuple"}), | ||
| "bytes": frozenset(), |
There was a problem hiding this comment.
bytes is listed as a supported builtin annotation but has no accepted literal kind, and byte literals are not classified. As a result, a valid default such as b"data" cannot be used for a bytes parameter, blocking valid signature changes.
Artifacts
- The reproduction evaluates a bytes literal against the new compatibility helper.
- The captured source shows the exact bytes literal and annotation evaluated by the reproduction.
- The output reports `literal_kind=None` and `accepted=False`, confirming that the valid bytes default is rejected.
| def _compatible(kind: str, annotation: str) -> bool: | ||
| """Whether a literal of `kind` fits `annotation` (builtin names only).""" | ||
| text = annotation.strip() | ||
| if kind == "None": |
There was a problem hiding this comment.
The early None branch rejects None before broad annotations reach the conservative unknown-type fallback. Valid defaults for Any and object are therefore refused, preventing otherwise valid nullable signature changes.
Artifacts
Broad null-default reproduction
- The reproduction evaluates a `None` literal with broad annotations including `Any` and `object`.
- The output shows `None` is rejected for `Any` and the expected compatibility assertion fails.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved MCP registration and literal/spec validation issues block approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
Adds foundational types, constants, parsing, and literal/type validation for the planned change_signature operation.
Changes:
- Adds signature specifications, reports, and site result types.
- Adds literal/type compatibility helpers.
- Adds AST, CLI, graph, and MCP constants.
| File | Summary |
|---|---|
codebase_rag/editing/signature_types.py |
Defines signature reports, parameter specifications, and parsing. |
codebase_rag/editing/signature_literals.py |
Adds literal/type compatibility validation. |
codebase_rag/constants/mcp.py |
Adds change-signature MCP identifiers. |
codebase_rag/constants/graph.py |
Adds graph report keys. |
codebase_rag/constants/cli.py |
Adds signature-operation messages. |
codebase_rag/constants/ast_python.py |
Adds Python call-splat constants. |
codebase_rag/constants/ast_nodes.py |
Adds the spread-element node constant. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| TESTS_REACHING = "tests_reaching" | ||
| # Graph-driven edit operations (issue #1532). | ||
| RENAME = "rename" | ||
| CHANGE_SIGNATURE = "change_signature" |
| _KIND_OF = { | ||
| bool: "bool", | ||
| int: "int", | ||
| float: "float", | ||
| str: "str", | ||
| list: "list", | ||
| dict: "dict", | ||
| set: "set", | ||
| tuple: "tuple", | ||
| type(None): "None", | ||
| } |
| r"(?:=(?P<default>[^@]+))?" | ||
| r"(?:@(?P<source>.+))?$" |
Public report/spec types, shared constants, and literal/type validation helpers.
Files changed
codebase_rag/constants/ast_nodes.pycodebase_rag/constants/ast_python.pycodebase_rag/constants/cli.pycodebase_rag/constants/graph.pycodebase_rag/constants/mcp.pycodebase_rag/editing/signature_types.pycodebase_rag/editing/signature_literals.pyDiff stats
+211 additions, -0 deletions (211 LOC)
Dependency graph
Merge in this order:
Summary by CodeRabbit