Found during the architecture audit (sub-agent review).
Finding (HIGH)
graphforge-cypher is described as "Cypher lexer and Pratt parser" but is a covert orchestrator reaching two stages downstream:
crates/graphforge-cypher/Cargo.toml:14 — production dep on graphforge-rel (and graphforge-ir)
cypher/src/lib.rs:16 — re-exports Binder, OntologyMode, RuntimeCatalog
cypher/src/lib.rs:79 — calls graphforge_rel::explain_logical for ExplainStage::LogicalPlan
cypher/src/lib.rs:96-120 — explain_stage drives binding itself
core/src/lib.rs:950-953 — documents this was done "to avoid circular crate dependencies"
This creates a second front-door that bypasses graphforge-api (the declared public API) and drags DataFusion/rel into the parser's dep tree.
Related inconsistencies:
- Same
BindError maps to GfError::Bind via api (api:3646-3665) but GfError::Plan via cypher::explain_stage (cypher/lib.rs:99-102) — identical binder rejection yields different public error codes depending on entry point
cypher/src/lib.rs:5-7 — crate is exempt from workspace lints (allow(clippy::all), allow(clippy::pedantic)) while every other pipeline crate inherits lints.workspace = true
Acceptance criteria
Architecture review decision
The finding is confirmed, with one wording correction: Cargo.toml already declares [lints] workspace = true; the crate-level #![allow(clippy::all)] and #![allow(clippy::pedantic)] directives neutralize those checks.
Dependency: define the facade error conversion in #1018 before declaring binder-code parity complete.
Additional acceptance criteria:
Found during the architecture audit (sub-agent review).
Finding (HIGH)
graphforge-cypheris described as "Cypher lexer and Pratt parser" but is a covert orchestrator reaching two stages downstream:crates/graphforge-cypher/Cargo.toml:14— production dep ongraphforge-rel(andgraphforge-ir)cypher/src/lib.rs:16— re-exportsBinder,OntologyMode,RuntimeCatalogcypher/src/lib.rs:79— callsgraphforge_rel::explain_logicalforExplainStage::LogicalPlancypher/src/lib.rs:96-120—explain_stagedrives binding itselfcore/src/lib.rs:950-953— documents this was done "to avoid circular crate dependencies"This creates a second front-door that bypasses
graphforge-api(the declared public API) and drags DataFusion/rel into the parser's dep tree.Related inconsistencies:
BindErrormaps toGfError::Bindvia api (api:3646-3665) butGfError::Planviacypher::explain_stage(cypher/lib.rs:99-102) — identical binder rejection yields different public error codes depending on entry pointcypher/src/lib.rs:5-7— crate is exempt from workspace lints (allow(clippy::all),allow(clippy::pedantic)) while every other pipeline crate inheritslints.workspace = trueAcceptance criteria
graphforge-cypherproduction deps limited tographforge-ast(+graphforge-core)ExplainStage::LogicalPlanorchestration moves tographforge-api(already depends on both stages)Architecture review decision
The finding is confirmed, with one wording correction:
Cargo.tomlalready declares[lints] workspace = true; the crate-level#![allow(clippy::all)]and#![allow(clippy::pedantic)]directives neutralize those checks.Dependency: define the facade error conversion in #1018 before declaring binder-code parity complete.
Additional acceptance criteria:
graphforge-api