chore: remove deprecated Python parser/planner/executor stack#662
Merged
Conversation
Delete the entire Python 0.4.x pipeline — the Rust recursive-descent +
Pratt parser is the authoritative implementation going forward, and the
TCK is driven by the Rust cucumber-rs BDD runner.
Removed:
- src/graphforge/{api.py,ast/,parser/,planner/,optimizer/,executor/}
- src/graphforge/{storage/,search/,algorithms/,datasets/,procedures/,types/,_compat.py}
- tests/unit/{parser,ast,planner,optimizer,executor,storage,search,algorithms,datasets,procedures}/
- tests/unit/api/{test_api_coverage,test_bulk_ingest,test_clear,test_cypher_value_conversion,test_json_io,test_add_graph_documents,test_analytics,test_cache,test_clone,test_merge_node,test_schema_introspection,test_set_node_properties}.py
- tests/{tck/,parser_parity/}
- crates/gf-cypher/src/bin/parse_json.rs (parity harness binary)
Kept and promoted:
- src/graphforge/api_v05.py → src/graphforge/api.py (v0.5 Arrow-returning stubs)
- src/graphforge/{exceptions.py,recipes/}
- tests/unit/api/test_api_v05.py → tests the v0.5 surface
- tests/features/ — BDD scenarios driving the Rust implementation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
WalkthroughReplaces the production GraphForge (v0.4) with a v0.5 test stub: adds in-memory Node/Edge handles and PyArrow-returning stub methods, and removes parser/planner/optimizer, storage, dataset/format/exporter, algorithm, and many parser-parity tests. ChangesGraphForge v0.5 Stub Migration
🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Comment on lines
+371
to
+374
| rows = list(records) | ||
| except Exception: | ||
| rows = [] | ||
| node_map = {n.id: n for n in self._nodes} |
There was a problem hiding this comment.
🟠 High graphforge/api.py:371
add_edges calls list(records) directly on line 371, which for a pyarrow.Table iterates over column names instead of rows, causing the method to silently produce no edges when passed tabular data. Unlike add_nodes, it doesn't handle pyarrow.Table, pandas DataFrame, or polars DataFrame inputs. Consider adding the same conversion logic used in add_nodes (lines 343-355) to extract rows before processing.
- try:
- rows = list(records)
- except Exception:
- rows = []
+ try:
+ import pyarrow as pa
+
+ if isinstance(records, pa.Table):
+ rows = records.to_pylist()
+ elif hasattr(records, "to_dict"):
+ rows = records.to_dict("records") # pandas DataFrame
+ elif hasattr(records, "to_dicts"):
+ rows = records.to_dicts() # polars DataFrame
+ else:
+ rows = list(records)
+ except Exception:
+ rows = []🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file src/graphforge/api.py around lines 371-374:
`add_edges` calls `list(records)` directly on line 371, which for a `pyarrow.Table` iterates over column names instead of rows, causing the method to silently produce no edges when passed tabular data. Unlike `add_nodes`, it doesn't handle `pyarrow.Table`, pandas DataFrame, or polars DataFrame inputs. Consider adding the same conversion logic used in `add_nodes` (lines 343-355) to extract rows before processing.
Evidence trail:
src/graphforge/api.py lines 338-362 (add_nodes with conversion logic for pyarrow, pandas, polars), lines 364-385 (add_edges without such logic, line 371: `rows = list(records)`). pyarrow.Table.__iter__ yields column names (strings), not row dicts, causing isinstance(row, dict) check at line 376 to fail silently.
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/graphforge/__init__.py (1)
17-28:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing
SearchHitexport/definition breaksfrom graphforge import SearchHit.
tests/integration/test_three_surfaces.pyimportsSearchHitfrom the packagesrc/graphforge/__init__.py’s__all__doesn’t exportSearchHit, and there is noSearchHitsymbol anywhere undersrc/graphforge/Update the package to provide
SearchHit(and re-export it fromsrc/graphforge/__init__.py), or adjust the consuming tests to the new result type/location.🤖 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 `@src/graphforge/__init__.py` around lines 17 - 28, The package is missing the SearchHit symbol which breaks imports; add a SearchHit definition (e.g., a lightweight dataclass or namedtuple representing search result fields) in an appropriate module (for example create or extend a module like results.py or types.py) and then re-export it from the package root by adding "SearchHit" to the __all__ list in __init__ and importing it into __init__ (e.g., from .results import SearchHit). Ensure the new SearchHit type matches how tests use it (fields/methods) and update any consuming code/tests to import the new location if you choose to keep it outside the package root.
🤖 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 `@src/graphforge/api.py`:
- Around line 239-242: The current validation in GraphForge.__init__ (checking
self._path and raising StorageError) prevents auto-creation of the workspace
directory and breaks tests; change the logic in the constructor where self._path
is validated so that if self._path is not None and does not exist you create the
directory (use Path.mkdir(parents=True, exist_ok=True)) instead of raising
StorageError, and keep the existing check that raises StorageError if the path
exists but is not a directory; ensure the behavior aligns with prior v0.4
auto-create semantics for GraphForge(path).
- Around line 391-419: The parse-error simulation in execute is incorrectly
treating "NOT" as a valid starting keyword; remove "NOT" from the known_keywords
set (or otherwise exclude it from the accepted openCypher starts) so that
queries like "NOT VALID CYPHER !!!" trigger the intended ParseError; update the
known_keywords set used in execute (and leave _validate_query, ParseError, and
_empty_execute logic unchanged) so first_word checks behave correctly.
In `@src/graphforge/recipes/__init__.py`:
- Around line 9-11: The import typing.Any is unused and causes an F401 lint
error; update the import statement to remove Any so it reads "from typing import
TYPE_CHECKING" (leaving the existing TYPE_CHECKING usage intact) or otherwise
use Any where intended; specifically edit the top-level import line that
currently includes Any to drop that symbol.
---
Outside diff comments:
In `@src/graphforge/__init__.py`:
- Around line 17-28: The package is missing the SearchHit symbol which breaks
imports; add a SearchHit definition (e.g., a lightweight dataclass or namedtuple
representing search result fields) in an appropriate module (for example create
or extend a module like results.py or types.py) and then re-export it from the
package root by adding "SearchHit" to the __all__ list in __init__ and importing
it into __init__ (e.g., from .results import SearchHit). Ensure the new
SearchHit type matches how tests use it (fields/methods) and update any
consuming code/tests to import the new location if you choose to keep it outside
the package root.
🪄 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: d1da3c04-7b0f-4f48-9ba0-ee3cdcbf8232
📒 Files selected for processing (300)
.claude/worktrees/agent-a9d3563dc802c115ecrates/gf-cypher/Cargo.tomlcrates/gf-cypher/src/bin/parse_json.rssrc/graphforge/__init__.pysrc/graphforge/_compat.pysrc/graphforge/algorithms/__init__.pysrc/graphforge/algorithms/_dispatch.pysrc/graphforge/algorithms/centrality.pysrc/graphforge/algorithms/community.pysrc/graphforge/algorithms/gds.pysrc/graphforge/algorithms/structural.pysrc/graphforge/api.pysrc/graphforge/api_v05.pysrc/graphforge/ast/__init__.pysrc/graphforge/ast/clause.pysrc/graphforge/ast/expression.pysrc/graphforge/ast/pattern.pysrc/graphforge/ast/query.pysrc/graphforge/datasets/__init__.pysrc/graphforge/datasets/base.pysrc/graphforge/datasets/data/__init__.pysrc/graphforge/datasets/data/networkrepository.jsonsrc/graphforge/datasets/data/snap.jsonsrc/graphforge/datasets/exporters/__init__.pysrc/graphforge/datasets/exporters/json_graph.pysrc/graphforge/datasets/formats/__init__.pysrc/graphforge/datasets/formats/json_graph.pysrc/graphforge/datasets/loaders/__init__.pysrc/graphforge/datasets/loaders/compression.pysrc/graphforge/datasets/loaders/csv.pysrc/graphforge/datasets/loaders/cypher.pysrc/graphforge/datasets/loaders/graphml.pysrc/graphforge/datasets/loaders/json_graph.pysrc/graphforge/datasets/loaders/ldbc.pysrc/graphforge/datasets/loaders/ldbc_schema.pysrc/graphforge/datasets/registry.pysrc/graphforge/datasets/sources/__init__.pysrc/graphforge/datasets/sources/graphml.pysrc/graphforge/datasets/sources/json_graph.pysrc/graphforge/datasets/sources/ldbc.pysrc/graphforge/datasets/sources/networkrepository.pysrc/graphforge/datasets/sources/snap.pysrc/graphforge/executor/__init__.pysrc/graphforge/executor/evaluator.pysrc/graphforge/executor/executor.pysrc/graphforge/optimizer/__init__.pysrc/graphforge/optimizer/cost_model.pysrc/graphforge/optimizer/join_reorder.pysrc/graphforge/optimizer/optimizer.pysrc/graphforge/optimizer/predicate_utils.pysrc/graphforge/optimizer/statistics.pysrc/graphforge/parser/__init__.pysrc/graphforge/parser/cypher.larksrc/graphforge/parser/parser.pysrc/graphforge/planner/__init__.pysrc/graphforge/planner/operators.pysrc/graphforge/planner/planner.pysrc/graphforge/planner/types.pysrc/graphforge/procedures/__init__.pysrc/graphforge/procedures/registry.pysrc/graphforge/recipes/__init__.pysrc/graphforge/search/__init__.pysrc/graphforge/search/search.pysrc/graphforge/search/types.pysrc/graphforge/storage/__init__.pysrc/graphforge/storage/memory.pysrc/graphforge/storage/pydantic_serialization.pysrc/graphforge/storage/serialization.pysrc/graphforge/storage/sqlite_backend.pysrc/graphforge/types/__init__.pysrc/graphforge/types/graph.pysrc/graphforge/types/values.pytests/conftest.pytests/features/steps/api_steps.pytests/parser_parity/__init__.pytests/parser_parity/corpus.pytests/parser_parity/harness.pytests/parser_parity/test_parity.pytests/tck/__init__.pytests/tck/conftest.pytests/tck/coverage_matrix.jsontests/tck/download_tck.pytests/tck/features/Aggregation1_BasicAggregation.featuretests/tck/features/Match1_SimpleMatches.featuretests/tck/features/official/clauses/call/Call1.featuretests/tck/features/official/clauses/call/Call2.featuretests/tck/features/official/clauses/call/Call3.featuretests/tck/features/official/clauses/call/Call4.featuretests/tck/features/official/clauses/call/Call5.featuretests/tck/features/official/clauses/call/Call6.featuretests/tck/features/official/clauses/create/Create1.featuretests/tck/features/official/clauses/create/Create2.featuretests/tck/features/official/clauses/create/Create3.featuretests/tck/features/official/clauses/create/Create4.featuretests/tck/features/official/clauses/create/Create5.featuretests/tck/features/official/clauses/create/Create6.featuretests/tck/features/official/clauses/delete/Delete1.featuretests/tck/features/official/clauses/delete/Delete2.featuretests/tck/features/official/clauses/delete/Delete3.featuretests/tck/features/official/clauses/delete/Delete4.featuretests/tck/features/official/clauses/delete/Delete5.featuretests/tck/features/official/clauses/delete/Delete6.featuretests/tck/features/official/clauses/match-where/MatchWhere1.featuretests/tck/features/official/clauses/match-where/MatchWhere2.featuretests/tck/features/official/clauses/match-where/MatchWhere3.featuretests/tck/features/official/clauses/match-where/MatchWhere4.featuretests/tck/features/official/clauses/match-where/MatchWhere5.featuretests/tck/features/official/clauses/match-where/MatchWhere6.featuretests/tck/features/official/clauses/match/Match1.featuretests/tck/features/official/clauses/match/Match2.featuretests/tck/features/official/clauses/match/Match3.featuretests/tck/features/official/clauses/match/Match4.featuretests/tck/features/official/clauses/match/Match5.featuretests/tck/features/official/clauses/match/Match6.featuretests/tck/features/official/clauses/match/Match7.featuretests/tck/features/official/clauses/match/Match8.featuretests/tck/features/official/clauses/match/Match9.featuretests/tck/features/official/clauses/merge/Merge1.featuretests/tck/features/official/clauses/merge/Merge2.featuretests/tck/features/official/clauses/merge/Merge3.featuretests/tck/features/official/clauses/merge/Merge4.featuretests/tck/features/official/clauses/merge/Merge5.featuretests/tck/features/official/clauses/merge/Merge6.featuretests/tck/features/official/clauses/merge/Merge7.featuretests/tck/features/official/clauses/merge/Merge8.featuretests/tck/features/official/clauses/merge/Merge9.featuretests/tck/features/official/clauses/remove/Remove1.featuretests/tck/features/official/clauses/remove/Remove2.featuretests/tck/features/official/clauses/remove/Remove3.featuretests/tck/features/official/clauses/return-orderby/ReturnOrderBy1.featuretests/tck/features/official/clauses/return-orderby/ReturnOrderBy2.featuretests/tck/features/official/clauses/return-orderby/ReturnOrderBy3.featuretests/tck/features/official/clauses/return-orderby/ReturnOrderBy4.featuretests/tck/features/official/clauses/return-orderby/ReturnOrderBy5.featuretests/tck/features/official/clauses/return-orderby/ReturnOrderBy6.featuretests/tck/features/official/clauses/return-skip-limit/ReturnSkipLimit1.featuretests/tck/features/official/clauses/return-skip-limit/ReturnSkipLimit2.featuretests/tck/features/official/clauses/return-skip-limit/ReturnSkipLimit3.featuretests/tck/features/official/clauses/return/Return1.featuretests/tck/features/official/clauses/return/Return2.featuretests/tck/features/official/clauses/return/Return3.featuretests/tck/features/official/clauses/return/Return4.featuretests/tck/features/official/clauses/return/Return5.featuretests/tck/features/official/clauses/return/Return6.featuretests/tck/features/official/clauses/return/Return7.featuretests/tck/features/official/clauses/return/Return8.featuretests/tck/features/official/clauses/set/Set1.featuretests/tck/features/official/clauses/set/Set2.featuretests/tck/features/official/clauses/set/Set3.featuretests/tck/features/official/clauses/set/Set4.featuretests/tck/features/official/clauses/set/Set5.featuretests/tck/features/official/clauses/set/Set6.featuretests/tck/features/official/clauses/union/Union1.featuretests/tck/features/official/clauses/union/Union2.featuretests/tck/features/official/clauses/union/Union3.featuretests/tck/features/official/clauses/unwind/Unwind1.featuretests/tck/features/official/clauses/with-orderBy/WithOrderBy1.featuretests/tck/features/official/clauses/with-orderBy/WithOrderBy2.featuretests/tck/features/official/clauses/with-orderBy/WithOrderBy3.featuretests/tck/features/official/clauses/with-orderBy/WithOrderBy4.featuretests/tck/features/official/clauses/with-skip-limit/WithSkipLimit1.featuretests/tck/features/official/clauses/with-skip-limit/WithSkipLimit2.featuretests/tck/features/official/clauses/with-skip-limit/WithSkipLimit3.featuretests/tck/features/official/clauses/with-where/WithWhere1.featuretests/tck/features/official/clauses/with-where/WithWhere2.featuretests/tck/features/official/clauses/with-where/WithWhere3.featuretests/tck/features/official/clauses/with-where/WithWhere4.featuretests/tck/features/official/clauses/with-where/WithWhere5.featuretests/tck/features/official/clauses/with-where/WithWhere6.featuretests/tck/features/official/clauses/with-where/WithWhere7.featuretests/tck/features/official/clauses/with/With1.featuretests/tck/features/official/clauses/with/With2.featuretests/tck/features/official/clauses/with/With3.featuretests/tck/features/official/clauses/with/With4.featuretests/tck/features/official/clauses/with/With5.featuretests/tck/features/official/clauses/with/With6.featuretests/tck/features/official/clauses/with/With7.featuretests/tck/features/official/expressions/aggregation/Aggregation1.featuretests/tck/features/official/expressions/aggregation/Aggregation2.featuretests/tck/features/official/expressions/aggregation/Aggregation3.featuretests/tck/features/official/expressions/aggregation/Aggregation4.featuretests/tck/features/official/expressions/aggregation/Aggregation5.featuretests/tck/features/official/expressions/aggregation/Aggregation6.featuretests/tck/features/official/expressions/aggregation/Aggregation7.featuretests/tck/features/official/expressions/aggregation/Aggregation8.featuretests/tck/features/official/expressions/boolean/Boolean1.featuretests/tck/features/official/expressions/boolean/Boolean2.featuretests/tck/features/official/expressions/boolean/Boolean3.featuretests/tck/features/official/expressions/boolean/Boolean4.featuretests/tck/features/official/expressions/boolean/Boolean5.featuretests/tck/features/official/expressions/comparison/Comparison1.featuretests/tck/features/official/expressions/comparison/Comparison2.featuretests/tck/features/official/expressions/comparison/Comparison3.featuretests/tck/features/official/expressions/comparison/Comparison4.featuretests/tck/features/official/expressions/conditional/Conditional1.featuretests/tck/features/official/expressions/conditional/Conditional2.featuretests/tck/features/official/expressions/existentialSubqueries/ExistentialSubquery1.featuretests/tck/features/official/expressions/existentialSubqueries/ExistentialSubquery2.featuretests/tck/features/official/expressions/existentialSubqueries/ExistentialSubquery3.featuretests/tck/features/official/expressions/graph/Graph1.featuretests/tck/features/official/expressions/graph/Graph2.featuretests/tck/features/official/expressions/graph/Graph3.featuretests/tck/features/official/expressions/graph/Graph4.featuretests/tck/features/official/expressions/graph/Graph5.featuretests/tck/features/official/expressions/graph/Graph6.featuretests/tck/features/official/expressions/graph/Graph7.featuretests/tck/features/official/expressions/graph/Graph8.featuretests/tck/features/official/expressions/graph/Graph9.featuretests/tck/features/official/expressions/list/List1.featuretests/tck/features/official/expressions/list/List10.featuretests/tck/features/official/expressions/list/List11.featuretests/tck/features/official/expressions/list/List12.featuretests/tck/features/official/expressions/list/List2.featuretests/tck/features/official/expressions/list/List3.featuretests/tck/features/official/expressions/list/List4.featuretests/tck/features/official/expressions/list/List5.featuretests/tck/features/official/expressions/list/List6.featuretests/tck/features/official/expressions/list/List7.featuretests/tck/features/official/expressions/list/List8.featuretests/tck/features/official/expressions/list/List9.featuretests/tck/features/official/expressions/literals/Literals1.featuretests/tck/features/official/expressions/literals/Literals2.featuretests/tck/features/official/expressions/literals/Literals3.featuretests/tck/features/official/expressions/literals/Literals4.featuretests/tck/features/official/expressions/literals/Literals5.featuretests/tck/features/official/expressions/literals/Literals6.featuretests/tck/features/official/expressions/literals/Literals7.featuretests/tck/features/official/expressions/literals/Literals8.featuretests/tck/features/official/expressions/map/Map1.featuretests/tck/features/official/expressions/map/Map2.featuretests/tck/features/official/expressions/map/Map3.featuretests/tck/features/official/expressions/mathematical/Mathematical1.featuretests/tck/features/official/expressions/mathematical/Mathematical10.featuretests/tck/features/official/expressions/mathematical/Mathematical11.featuretests/tck/features/official/expressions/mathematical/Mathematical12.featuretests/tck/features/official/expressions/mathematical/Mathematical13.featuretests/tck/features/official/expressions/mathematical/Mathematical14.featuretests/tck/features/official/expressions/mathematical/Mathematical15.featuretests/tck/features/official/expressions/mathematical/Mathematical16.featuretests/tck/features/official/expressions/mathematical/Mathematical17.featuretests/tck/features/official/expressions/mathematical/Mathematical2.featuretests/tck/features/official/expressions/mathematical/Mathematical3.featuretests/tck/features/official/expressions/mathematical/Mathematical4.featuretests/tck/features/official/expressions/mathematical/Mathematical5.featuretests/tck/features/official/expressions/mathematical/Mathematical6.featuretests/tck/features/official/expressions/mathematical/Mathematical7.featuretests/tck/features/official/expressions/mathematical/Mathematical8.featuretests/tck/features/official/expressions/mathematical/Mathematical9.featuretests/tck/features/official/expressions/null/Null1.featuretests/tck/features/official/expressions/null/Null2.featuretests/tck/features/official/expressions/null/Null3.featuretests/tck/features/official/expressions/path/Path1.featuretests/tck/features/official/expressions/path/Path2.featuretests/tck/features/official/expressions/path/Path3.featuretests/tck/features/official/expressions/pattern/Pattern1.featuretests/tck/features/official/expressions/pattern/Pattern2.featuretests/tck/features/official/expressions/precedence/Precedence1.featuretests/tck/features/official/expressions/precedence/Precedence2.featuretests/tck/features/official/expressions/precedence/Precedence3.featuretests/tck/features/official/expressions/precedence/Precedence4.featuretests/tck/features/official/expressions/quantifier/Quantifier1.featuretests/tck/features/official/expressions/quantifier/Quantifier10.featuretests/tck/features/official/expressions/quantifier/Quantifier11.featuretests/tck/features/official/expressions/quantifier/Quantifier12.featuretests/tck/features/official/expressions/quantifier/Quantifier2.featuretests/tck/features/official/expressions/quantifier/Quantifier3.featuretests/tck/features/official/expressions/quantifier/Quantifier4.featuretests/tck/features/official/expressions/quantifier/Quantifier5.featuretests/tck/features/official/expressions/quantifier/Quantifier6.featuretests/tck/features/official/expressions/quantifier/Quantifier7.featuretests/tck/features/official/expressions/quantifier/Quantifier8.featuretests/tck/features/official/expressions/quantifier/Quantifier9.featuretests/tck/features/official/expressions/string/String1.featuretests/tck/features/official/expressions/string/String10.featuretests/tck/features/official/expressions/string/String11.featuretests/tck/features/official/expressions/string/String12.featuretests/tck/features/official/expressions/string/String13.featuretests/tck/features/official/expressions/string/String14.featuretests/tck/features/official/expressions/string/String2.featuretests/tck/features/official/expressions/string/String3.featuretests/tck/features/official/expressions/string/String4.featuretests/tck/features/official/expressions/string/String5.featuretests/tck/features/official/expressions/string/String6.featuretests/tck/features/official/expressions/string/String7.featuretests/tck/features/official/expressions/string/String8.featuretests/tck/features/official/expressions/string/String9.featuretests/tck/features/official/expressions/temporal/Temporal1.featuretests/tck/features/official/expressions/temporal/Temporal10.featuretests/tck/features/official/expressions/temporal/Temporal2.featuretests/tck/features/official/expressions/temporal/Temporal3.featuretests/tck/features/official/expressions/temporal/Temporal4.featuretests/tck/features/official/expressions/temporal/Temporal5.featuretests/tck/features/official/expressions/temporal/Temporal6.featuretests/tck/features/official/expressions/temporal/Temporal7.featuretests/tck/features/official/expressions/temporal/Temporal8.featuretests/tck/features/official/expressions/temporal/Temporal9.featuretests/tck/features/official/expressions/typeConversion/TypeConversion1.featuretests/tck/features/official/expressions/typeConversion/TypeConversion2.featuretests/tck/features/official/expressions/typeConversion/TypeConversion3.featuretests/tck/features/official/expressions/typeConversion/TypeConversion4.feature
💤 Files with no reviewable changes (70)
- crates/gf-cypher/src/bin/parse_json.rs
- src/graphforge/parser/cypher.lark
- src/graphforge/datasets/data/init.py
- src/graphforge/types/values.py
- src/graphforge/datasets/data/snap.json
- src/graphforge/_compat.py
- src/graphforge/algorithms/community.py
- src/graphforge/storage/memory.py
- tests/tck/init.py
- src/graphforge/datasets/loaders/init.py
- src/graphforge/procedures/init.py
- src/graphforge/datasets/sources/json_graph.py
- src/graphforge/datasets/sources/graphml.py
- tests/parser_parity/test_parity.py
- src/graphforge/optimizer/statistics.py
- src/graphforge/algorithms/gds.py
- src/graphforge/datasets/base.py
- src/graphforge/storage/serialization.py
- src/graphforge/datasets/init.py
- src/graphforge/datasets/formats/json_graph.py
- src/graphforge/planner/init.py
- src/graphforge/parser/init.py
- src/graphforge/datasets/sources/init.py
- src/graphforge/datasets/sources/ldbc.py
- src/graphforge/datasets/sources/snap.py
- src/graphforge/datasets/exporters/json_graph.py
- src/graphforge/datasets/loaders/compression.py
- src/graphforge/executor/init.py
- src/graphforge/ast/init.py
- src/graphforge/algorithms/structural.py
- src/graphforge/optimizer/cost_model.py
- src/graphforge/search/search.py
- src/graphforge/search/init.py
- src/graphforge/datasets/loaders/ldbc_schema.py
- src/graphforge/search/types.py
- src/graphforge/datasets/loaders/csv.py
- src/graphforge/datasets/data/networkrepository.json
- src/graphforge/datasets/loaders/cypher.py
- src/graphforge/planner/types.py
- tests/conftest.py
- src/graphforge/procedures/registry.py
- tests/parser_parity/corpus.py
- src/graphforge/ast/clause.py
- src/graphforge/ast/query.py
- src/graphforge/types/init.py
- src/graphforge/datasets/sources/networkrepository.py
- src/graphforge/datasets/loaders/ldbc.py
- src/graphforge/ast/pattern.py
- src/graphforge/storage/sqlite_backend.py
- src/graphforge/datasets/formats/init.py
- src/graphforge/storage/pydantic_serialization.py
- src/graphforge/algorithms/_dispatch.py
- src/graphforge/types/graph.py
- src/graphforge/optimizer/predicate_utils.py
- src/graphforge/datasets/loaders/graphml.py
- src/graphforge/datasets/registry.py
- src/graphforge/ast/expression.py
- crates/gf-cypher/Cargo.toml
- src/graphforge/datasets/loaders/json_graph.py
- src/graphforge/algorithms/centrality.py
- src/graphforge/parser/parser.py
- src/graphforge/optimizer/join_reorder.py
- tests/parser_parity/harness.py
- src/graphforge/planner/operators.py
- src/graphforge/algorithms/init.py
- src/graphforge/api_v05.py
- src/graphforge/optimizer/init.py
- src/graphforge/storage/init.py
- src/graphforge/optimizer/optimizer.py
- src/graphforge/datasets/exporters/init.py
Comment on lines
+239
to
+242
| if self._path is not None and not self._path.exists(): | ||
| raise StorageError(f"Path does not exist: {self._path}") | ||
| if self._path is not None and self._path.exists() and not self._path.is_dir(): | ||
| raise StorageError(f"Path must be a directory, not a file: {self._path}") |
Contributor
There was a problem hiding this comment.
Path validation prevents auto-creation, breaking existing tests.
The pipeline fails because GraphForge(path) now requires the directory to exist, but tests expect auto-creation. Consider aligning with v0.4 behavior or documenting this breaking change.
🛠️ Option: Create directory if it doesn't exist
if self._path is not None and not self._path.exists():
- raise StorageError(f"Path does not exist: {self._path}")
+ try:
+ self._path.mkdir(parents=True, exist_ok=True)
+ except OSError as e:
+ raise StorageError(f"Cannot create path: {self._path}: {e}") from e
if self._path is not None and self._path.exists() and not self._path.is_dir():
raise StorageError(f"Path must be a directory, not a file: {self._path}")🧰 Tools
🪛 GitHub Actions: Test Suite / 19_Coverage Shard 1_4.txt
[error] 240-240: StorageError: Path does not exist: /tmp/tmpe9q8gqng/test.db (raised during GraphForge(db_path) initialization).
🪛 GitHub Actions: Test Suite / Coverage Shard 1_4
[error] 240-240: StorageError: Path does not exist: /tmp/tmpe9q8gqng/test.db (raised in GraphForge init)
🤖 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 `@src/graphforge/api.py` around lines 239 - 242, The current validation in
GraphForge.__init__ (checking self._path and raising StorageError) prevents
auto-creation of the workspace directory and breaks tests; change the logic in
the constructor where self._path is validated so that if self._path is not None
and does not exist you create the directory (use Path.mkdir(parents=True,
exist_ok=True)) instead of raising StorageError, and keep the existing check
that raises StorageError if the path exists but is not a directory; ensure the
behavior aligns with prior v0.4 auto-create semantics for GraphForge(path).
Comment on lines
+391
to
+419
| def execute(self, query: str, parameters: dict[str, Any] | None = None) -> pa.Table: | ||
| self._ensure_open() | ||
| if not self._in_transaction: | ||
| raise RuntimeError("Not in a transaction. Call begin() first.") | ||
| _validate_query(query) | ||
| # Stub parse-error simulation: any query that starts with a keyword | ||
| # not in the openCypher set raises ParseError. | ||
| first_word = query.strip().split()[0].upper() | ||
| known_keywords = { | ||
| "MATCH", | ||
| "CREATE", | ||
| "MERGE", | ||
| "RETURN", | ||
| "WITH", | ||
| "WHERE", | ||
| "SET", | ||
| "DELETE", | ||
| "DETACH", | ||
| "REMOVE", | ||
| "UNWIND", | ||
| "CALL", | ||
| "OPTIONAL", | ||
| "FOREACH", | ||
| "LOAD", | ||
| "EXPLAIN", | ||
| "PROFILE", | ||
| "NOT", | ||
| } | ||
| if first_word not in known_keywords: | ||
| raise ParseError(f"Unexpected token: {first_word!r}", span=(0, len(first_word))) | ||
| return _empty_execute() |
Contributor
There was a problem hiding this comment.
NOT in known_keywords may break parse-error simulation.
The BDD test execute raises ParseError on invalid Cypher uses "NOT VALID CYPHER !!!". Since "NOT" is in known_keywords, this query won't raise ParseError as expected. NOT isn't a valid openCypher query start.
🐛 Proposed fix
"OPTIONAL",
"FOREACH",
"LOAD",
"EXPLAIN",
"PROFILE",
- "NOT",
}🤖 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 `@src/graphforge/api.py` around lines 391 - 419, The parse-error simulation in
execute is incorrectly treating "NOT" as a valid starting keyword; remove "NOT"
from the known_keywords set (or otherwise exclude it from the accepted
openCypher starts) so that queries like "NOT VALID CYPHER !!!" trigger the
intended ParseError; update the known_keywords set used in execute (and leave
_validate_query, ParseError, and _empty_execute logic unchanged) so first_word
checks behave correctly.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #662 +/- ##
===========================================
+ Coverage 84.10% 97.08% +12.97%
===========================================
Files 49 2 -47
Lines 12942 274 -12668
Branches 3632 41 -3591
===========================================
- Hits 10885 266 -10619
+ Misses 1272 5 -1267
+ Partials 785 3 -782
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
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.
Summary
Removes the entire Python 0.4.x pipeline. The Rust recursive-descent + Pratt parser is the authoritative implementation; TCK compliance is driven by the Rust cucumber-rs BDD runner in
crates/gf-core/tests/bdd/.Deleted (~117k lines):
src/graphforge/{api.py, ast/, parser/, planner/, optimizer/, executor/, storage/, search/, algorithms/, datasets/, procedures/, types/, _compat.py}tests/unit/{parser, ast, planner, optimizer, executor, storage, search, algorithms, datasets, procedures}/tests/tck/— Python TCK runner (Rust BDD runner is the replacement)tests/parser_parity/— differential harness (milestone 9 complete)crates/gf-cypher/src/bin/parse_json.rs— only used by parity harnessKept and promoted:
src/graphforge/api_v05.py→src/graphforge/api.py(v0.5 Arrow-returning stubs)src/graphforge/{exceptions.py, recipes/}tests/unit/api/test_api_v05.py— tests the v0.5 API surfacetests/features/— BDD scenarios that drive the Rust implementationResult: 241 passing, 41 xfailed (pending Rust implementations), 0 failures.
🤖 Generated with Claude Code
Note
Remove deprecated Python parser/planner/executor stack from
graphforge.apisrc/graphforge/api.pywith a lightweight in-memory stub usingNodeHandleandEdgeHandleobjects.execute,rank,cluster,find,schema) now return empty typed pyarrow Tables;executeraisesParseErrorfor unrecognized leading keywords instead of running queries.begin/commit/rollback) is implemented as shallow in-memory snapshots; lifecycle guards raiseLifecycleErroron misuse.graphforge.recipes.neighbourhoodnow returns apyarrow.Tableviadb.execute()instead of alist[dict]viadb.to_dicts().__version__to0.5.0and removesSearchHit,unwrap,unwrap_results, andunwrap_rowfrom the top-levelgraphforgepackage exports.SearchHitand theunwrap*helpers are no longer importable fromgraphforge;neighbourhoodreturn type changes fromlist[dict]topyarrow.Table; no queries are actually executed against any backend.Macroscope summarized a270bfa.
Summary by CodeRabbit
Major Changes
Removed Features
Breaking Changes