feat: implement REMOVE clause for properties and labels

[DecisionNerd]

Summary

Implements issue #25: REMOVE clause for removing properties from nodes/relationships and labels from nodes.

Changes

Grammar (cypher.lark)

• Added remove_clause with support for property and label removal
• Updated update_query to support multiple SET/REMOVE clauses in any order
• Added ORDER BY/SKIP/LIMIT support after REMOVE

AST (ast/clause.py)

• Added RemoveClause class
• Added RemoveItem class with item_type ('property' or 'label')

Parser (parser.py)

• Added remove_clause() transformer
• Added remove_property() transformer
• Added remove_label() transformer

Planner (planner.py, operators.py)

• Added Remove operator
• Integrated REMOVE into query planning pipeline
• Positioned after SET, before DELETE in execution order

Executor (executor.py)

• Implemented _execute_remove() method
• Handles property removal via dict deletion
• Handles label removal by creating new NodeRef instances (immutability)
• Updates graph storage and execution context bindings

Features

• Remove single or multiple properties: REMOVE n.age, n.city
• Remove single or multiple labels: REMOVE n:Person, n:Employee
• Mix property and label removal: REMOVE n.age, n:Label
• Works with WHERE filtering
• Works with RETURN projection
• Compatible with SET and other update operations
• Properly handles NodeRef immutability

Tests

Added 16 comprehensive integration tests in test_remove_clause.py:

Property Removal Tests (5)

• Remove single property
• Remove multiple properties
• Remove nonexistent property (no error)
• Remove with RETURN
• Remove from multiple nodes

Label Removal Tests (3)

• Remove single label
• Remove nonexistent label (no error)
• Remove from multiple nodes

Mixed Tests (2)

• Remove property and label together
• Remove multiple properties and labels

Integration Tests (3)

• REMOVE with WHERE filter
• MATCH, REMOVE, then SET
• REMOVE with RETURN, ORDER BY, LIMIT

Edge Cases (3)

• Remove from relationship
• Remove all properties
• REMOVE with no matches

Test Results

• 723 tests passed, 14 skipped
• Coverage: 94.87% (above 85% threshold)
• All lint checks passed
• All type checks passed

Examples

Remove property:

MATCH (n:Person {name: 'Alice'})
REMOVE n.age

Remove label:

MATCH (n:Person:Employee {name: 'Bob'})
REMOVE n:Employee

Remove multiple properties and labels:

MATCH (n {name: 'Charlie'})
REMOVE n.age, n.city, n:Temp

Remove with filtering and ordering:

MATCH (n:Person)
WHERE n.age > 30
REMOVE n.flag
RETURN n.name
ORDER BY n.name

Notes

• NodeRef instances are immutable (frozen dataclass), so label removal creates new instances
• Property dicts are mutable, so property removal modifies in-place
• Grammar now allows multiple SET/REMOVE clauses in any order
• Closes feat: implement REMOVE clause for property/label removal #25

Checklist

• Grammar updated
• AST nodes created
• Parser transformers implemented
• Planner logic added
• Executor logic implemented
• 16 comprehensive tests added
• All tests passing (723 passed)
• Coverage maintained >85% (94.87%)
• Lint checks passed
• Type checks passed
• Pre-push checks passed

Generated with Claude Code

Summary by CodeRabbit

• New Features
  • REMOVE clause now available for removing properties and labels from nodes and relationships in Cypher queries
  • Support for combining REMOVE with WHERE, SET, and RETURN clauses in a single query
  • Idempotent property and label removal—removing non-existent items does not cause errors
  • Multiple properties and labels can be removed in a single operation

[coderabbitai]

Walkthrough

This PR implements the REMOVE clause feature, enabling removal of properties and labels from nodes and relationships. Changes span the Cypher parser (grammar and transformer), query planner (operator definition and planning logic), executor (removal implementation), and comprehensive test coverage across parser, planner, and integration layers.

Changes

Cohort / File(s)	Summary
AST Definitions src/graphforge/ast/clause.py	Added RemoveItem dataclass for single REMOVE targets (property or label) and RemoveClause dataclass containing a list of RemoveItem objects.
Parser Grammar & Transformer src/graphforge/parser/cypher.lark, src/graphforge/parser/parser.py	Added remove_clause and remove_item grammar rules. Introduced transformer methods (remove_clause, remove_property, remove_label) to construct RemoveClause and RemoveItem AST nodes from parsed syntax. Updated update_query rule to permit multiple set/remove operations and expanded unwind_query alternatives.
Planner Operator & Logic src/graphforge/planner/operators.py, src/graphforge/planner/planner.py	Added Remove operator dataclass holding a list of items. Integrated RemoveClause planning into _plan_simple_query to emit Remove operators before Delete in the execution plan.
Executor src/graphforge/executor/executor.py	Implemented _execute_remove to process RemoveItem objects: deletes properties from mutable elements (NodeRef/EdgeRef) and recreates NodeRef with updated labels for immutable label removal, updating graph and bindings accordingly.
Unit Tests: Parser tests/unit/parser/test_remove_parser.py	Added 12 parser unit tests covering single/multiple property removals, single/multiple label removals, mixed removals, and REMOVE interactions with WHERE, SET, RETURN, ORDER BY, and LIMIT clauses.
Unit Tests: Planner tests/unit/planner/test_remove_planner.py	Added 5 planner unit tests validating operator planning for property removal, multiple properties, label removal, mixed removals, and combined SET/REMOVE sequences.
Integration Tests tests/integration/test_remove_clause.py	Added 24 comprehensive integration tests covering property/label removal scenarios (single, multiple, non-existent, across multiple nodes), edge cases (relationships, all properties, no matches, idempotence), and interactions with WHERE, SET, RETURN, ORDER BY, and LIMIT clauses.

Sequence Diagram

sequenceDiagram
    participant Parser
    participant Planner
    participant Executor
    participant Graph

    Parser->>Parser: Parse REMOVE clause<br/>(variable.property or variable:Label)
    Parser->>Planner: Emit RemoveClause AST<br/>(contains list of RemoveItem)
    Planner->>Planner: Plan query: create Remove operator<br/>(from RemoveClause items)
    Executor->>Executor: _execute_remove(Remove, input_rows)
    loop For each input row and item
        alt Property Removal
            Executor->>Graph: Get element (NodeRef/EdgeRef)
            Graph-->>Executor: Return element with properties
            Executor->>Graph: Delete property from element
            Graph->>Graph: Update properties
        else Label Removal
            Executor->>Graph: Get NodeRef with labels
            Graph-->>Executor: Return NodeRef
            Executor->>Graph: Create new NodeRef<br/>(with updated labels)
            Graph->>Graph: Add new node to graph
            Executor->>Executor: Update binding to<br/>reference new NodeRef
        end
    end
    Executor-->>Executor: Return updated contexts

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main feature being implemented: the REMOVE clause for properties and labels. It directly aligns with the changeset's primary objective.
Description check	✅ Passed	The description is comprehensive and well-structured, covering changes across all major components (grammar, AST, parser, planner, executor), features, test results, examples, and a detailed checklist. While the template structure is not strictly followed in all sections, the content provided is thorough and complete.
Linked Issues check	✅ Passed	The PR successfully addresses all coding objectives from issue #25: grammar rules added, AST dataclasses created, parser transformers implemented, Remove operator added to planner, executor logic for property and label removal implemented, comprehensive tests added covering properties, labels, mixed scenarios, and edge cases.
Out of Scope Changes check	✅ Passed	All changes are within scope of issue #25. The grammar updates, AST additions, parser transformers, planner integration, executor implementation, and tests are all directly related to implementing REMOVE clause functionality as specified in the linked issue.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/25-remove-clause

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 95.83333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.68%. Comparing base (b82a727) to head (b1bc3bd).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
+ Coverage   92.59%   92.68%   +0.08%     
==========================================
  Files          15       15              
  Lines        1810     1858      +48     
  Branches      446      456      +10     
==========================================
+ Hits         1676     1722      +46     
  Misses         50       50              
- Partials       84       86       +2     

Flag	Coverage Δ
full-coverage	92.68% <95.83%> (+0.08%)	⬆️
unittests	49.94% <50.00%> (+1.82%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
parser	95.38% <100.00%> (+0.16%)	⬆️
planner	94.16% <100.00%> (+0.18%)	⬆️
executor	86.63% <91.30%> (+0.13%)	⬆️
storage	99.50% <ø> (ø)
ast	100.00% <100.00%> (ø)
types	98.42% <ø> (ø)

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b82a727...b1bc3bd. Read the comment docs.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/graphforge/parser/cypher.lark`:
- Around line 56-60: The grammar's remove_item currently only allows a single
label via "variable label", so update the remove_item rule to accept multiple
labels by replacing the second alternative with "variable labels" (keeping the
existing property_access -> remove_property and remove_label action names).
Modify the remove_item production so remove_clause, remove_item, remove_label,
labels and property_access are used consistently with the labels+ rule (same
pattern as node_pattern) to support REMOVE n:Person:Employee.

In `@src/graphforge/planner/planner.py`:
- Around line 108-109: The planner currently overwrites remove_clause for each
RemoveClause instance (the elif branch handling RemoveClause), discarding
earlier REMOVE clauses; change the collector to append each RemoveClause into a
list (e.g., remove_clauses) similar to
match_clauses/create_clauses/merge_clauses, and then update the
operator-building code that consumes remove_clause to iterate over
remove_clauses and construct/apply an operator per clause; ensure you update any
variable names and any code paths that expected a single remove_clause to handle
the list (e.g., places that referenced remove_clause when creating the
logical/remove operator).

🧹 Nitpick comments (1)

src/graphforge/ast/clause.py (1)

58-70: LGTM! Consider using an Enum for item_type for better type safety.

The RemoveItem dataclass is well-documented and follows the existing pattern. Using a string literal for item_type is pragmatic, but an Enum (e.g., RemoveItemType.PROPERTY, RemoveItemType.LABEL) would provide better IDE autocompletion and catch typos at parse time rather than runtime.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the full cypher.lark file around the remove_clause
cat -n src/graphforge/parser/cypher.lark | head -100

Repository: DecisionNerd/graphforge

Length of output: 4064

---

🏁 Script executed:

# Look for the label rules to understand how labels are defined
rg -n "^label" src/graphforge/parser/cypher.lark -A 2

Repository: DecisionNerd/graphforge

Length of output: 156

---

🏁 Script executed:

# Check if there's a plural 'labels' rule
rg -n "labels" src/graphforge/parser/cypher.lark

Repository: DecisionNerd/graphforge

Length of output: 140

---

🏁 Script executed:

# Look for the RemoveClause in clause.py to see the docstring
rg -n "class RemoveClause" src/graphforge/ast/clause.py -A 20

Repository: DecisionNerd/graphforge

Length of output: 556

---

🏁 Script executed:

# Search for tests with multi-label removal
rg -n "Person:Employee\|remove.*:" tests/ --type py -B 2 -A 2

Repository: DecisionNerd/graphforge

Length of output: 49

---

Grammar does not support multi-label removal syntax shown in docstring.

The remove_item rule uses variable label (singular), which only handles single label removal like REMOVE n:Person. However, the RemoveClause docstring in src/graphforge/ast/clause.py lists REMOVE n:Person:Employee as a valid example.

The grammar defines a labels: label+ rule that supports multiple labels, but remove_item doesn't use it. Update line 60 in src/graphforge/parser/cypher.lark:

remove_item: property_access          -> remove_property
           | variable labels          -> remove_label

This aligns the grammar with the documented syntax and the existing labels rule pattern used elsewhere (e.g., node_pattern).

🤖 Prompt for AI Agents

In `@src/graphforge/parser/cypher.lark` around lines 56 - 60, The grammar's
remove_item currently only allows a single label via "variable label", so update
the remove_item rule to accept multiple labels by replacing the second
alternative with "variable labels" (keeping the existing property_access ->
remove_property and remove_label action names). Modify the remove_item
production so remove_clause, remove_item, remove_label, labels and
property_access are used consistently with the labels+ rule (same pattern as
node_pattern) to support REMOVE n:Person:Employee.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/graphforge/planner/planner.py | sed -n '80,160p'

Repository: DecisionNerd/graphforge

Length of output: 3388

---

🏁 Script executed:

find . -type f -name "*.py" -o -name "*.lark" -o -name "*.g4" | grep -i "gram\|parse" | head -20

Repository: DecisionNerd/graphforge

Length of output: 359

---

🏁 Script executed:

cat src/graphforge/parser/cypher.lark | grep -A 5 -B 5 "remove_clause\|RemoveClause"

Repository: DecisionNerd/graphforge

Length of output: 1006

---

🏁 Script executed:

cat src/graphforge/parser/cypher.lark | head -40

Repository: DecisionNerd/graphforge

Length of output: 1782

---

🏁 Script executed:

cat src/graphforge/parser/test_remove_parser.py

Repository: DecisionNerd/graphforge

Length of output: 142

---

🏁 Script executed:

cat tests/unit/parser/test_remove_parser.py

Repository: DecisionNerd/graphforge

Length of output: 5218

---

🏁 Script executed:

rg -n "test.*multiple.*set.*clause" tests/ -A 10

Repository: DecisionNerd/graphforge

Length of output: 49

---

🏁 Script executed:

grep -r "SET.*SET" tests/unit/parser/ --include="*.py" -A 5 -B 5

Repository: DecisionNerd/graphforge

Length of output: 49

---

🏁 Script executed:

rg "MATCH.*SET.*SET" tests/

Repository: DecisionNerd/graphforge

Length of output: 49

---

🏁 Script executed:

cat tests/unit/parser/test_parser.py | grep -A 10 "def test"

Repository: DecisionNerd/graphforge

Length of output: 15544

---

🏁 Script executed:

grep -A 15 "test_parse_multiple_set" tests/unit/parser/test_parser.py

Repository: DecisionNerd/graphforge

Length of output: 49

---

🏁 Script executed:

rg "SET.*SET" tests/ -t py

Repository: DecisionNerd/graphforge

Length of output: 49

---

🏁 Script executed:

web_search
Does Neo4j Cypher support multiple REMOVE clauses in a single query

Repository: DecisionNerd/graphforge

Length of output: 160

---

🌐 Web query:

Does Neo4j Cypher support multiple REMOVE clauses in a single query

💡 Result:

Yes. Cypher queries are built by chaining clauses together, so you can use more than one REMOVE clause in the same query (e.g., ... REMOVE n.prop REMOVE n:Label ...). [1]

That said, you can often do it in a single REMOVE by removing multiple labels at once (e.g., REMOVE n:German:Swedish). [2]

Example

MATCH (n:Person {id: $id})
REMOVE n.tempFlag
REMOVE n:Deprecated
RETURN n;

Or (single REMOVE for multiple labels)

MATCH (n:Person {id: $id})
REMOVE n:Deprecated:Legacy
RETURN n;

Sources: [1] [2]

---

🏁 Script executed:

cat -n tests/unit/parser/test_remove_parser.py | sed -n '95,110p'

Repository: DecisionNerd/graphforge

Length of output: 753

---

🏁 Script executed:

grep -n "test_parse.*multiple.*set" tests/unit/parser/ -r -i

Repository: DecisionNerd/graphforge

Length of output: 49

---

Multiple REMOVE clauses are silently discarded — only the last one is preserved in the logical plan.

The grammar and parser correctly support multiple REMOVE clauses (e.g., MATCH (n) REMOVE n.age REMOVE n:Temp), and Neo4j Cypher officially supports this syntax. However, the planner's collector pattern overwrites remove_clause on each iteration (lines 108–109), keeping only the final clause. This differs from how match_clauses, create_clauses, and merge_clauses are handled—those use lists to preserve all occurrences.

Refactor to collect remove_clause instances into a list and iterate over all of them when building operators, matching the pattern used for other repeatable clauses.

🤖 Prompt for AI Agents

In `@src/graphforge/planner/planner.py` around lines 108 - 109, The planner
currently overwrites remove_clause for each RemoveClause instance (the elif
branch handling RemoveClause), discarding earlier REMOVE clauses; change the
collector to append each RemoveClause into a list (e.g., remove_clauses) similar
to match_clauses/create_clauses/merge_clauses, and then update the
operator-building code that consumes remove_clause to iterate over
remove_clauses and construct/apply an operator per clause; ensure you update any
variable names and any code paths that expected a single remove_clause to handle
the list (e.g., places that referenced remove_clause when creating the
logical/remove operator).