fix: SyntaxError validation — path variable conflicts, unicode escapes, MERGE var-length (#258)#398
Conversation
…s, MERGE var-length (#258) - Raise VariableAlreadyBound when a path variable name (p = ...) is also used as a node or relationship variable within the same pattern (e.g. MATCH p = (p)-[]->() or MATCH p = ()-[p]->()) - Raise InvalidUnicodeLiteral for malformed \\uXXXX escape sequences in string literals (e.g. RETURN '\\uH') - Raise CreatingVarLength when a variable-length relationship appears in MERGE (e.g. MERGE ()-[*]-()) - Remove lru_cache from _decode_cypher_string since it now raises Fixes 14 TCK scenarios: 3,823/3,885 passing (+14 vs prior state) Closes #258 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThe changes add compile-time validation for two openCypher specification requirements: detecting when a path variable name collides with node or relationship variables in the same MATCH pattern, and validating unicode escape sequences in string literals while rejecting variable-length relationships in MERGE operations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 46 minutes and 13 seconds.Comment |
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/graphforge/parser/parser.py`:
- Around line 74-85: The unicode escape handling block in parser.py (the branch
where nxt in ("u","U") that builds hex_part and calls chr()) currently validates
hex digits but not the Unicode scalar range; add an explicit check after parsing
hex_part (before calling chr()) to ensure the integer value is <= 0x10FFFF (and
>= 0), and if out of range raise the same SyntaxError("InvalidUnicodeLiteral:
Invalid unicode escape '\\{nxt}{hex_part}'"); keep updating
result.append(chr(codepoint)) and advancing i as before.
🪄 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: CHILL
Plan: Pro
Run ID: 59c78fc2-d4f9-4b7f-a8e4-444ea914be91
📒 Files selected for processing (2)
src/graphforge/parser/parser.pysrc/graphforge/planner/planner.py
❌ 39 Tests Failed:
View the top 3 failed test(s) by shortest run time
View the full list of 35 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
- Add 0x10FFFF upper-bound check on \uXXXX codepoint before calling chr(), raising InvalidUnicodeLiteral instead of letting Python raise ValueError - Move Full OpenCypher* footnote to bottom of Why GraphForge? section Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
VariableAlreadyBoundwhen a path variable name (e.g.p) is reused as a node or relationship variable within the sameMATCHpattern (MATCH p = (p)-[]->(),MATCH p = ()-[p]->())InvalidUnicodeLiteralfor malformed\uXXXXescape sequences in string literals (RETURN '\uH')CreatingVarLengthwhen a variable-length relationship appears inMERGE(MERGE ()-[*]-())lru_cachefrom_decode_cypher_stringsince it now raises exceptionsTest plan
Closes #258
🤖 Generated with Claude Code
Summary by CodeRabbit
\uXXXXand\UXXXXXXXX) with validation