035 graph query#62
Conversation
Introduces GraphQuery and Algorithms modules with supporting specs, refactoring Pattern.Graph into a structured submodule hierarchy.
There was a problem hiding this comment.
Pull request overview
This pull request introduces a new portable graph query interface (GraphQuery) to decouple graph algorithms from specific graph representations. The implementation adds two new modules (Pattern.Graph.GraphQuery and Pattern.Graph.Algorithms) that enable algorithms to work uniformly with both GraphLens and PatternGraph backends. The feature includes traversal direction and weight control at the call site, composable query transformations, and context query helpers.
Changes:
- Introduced
GraphQueryrecord-of-functions interface withTraversalDirectionandTraversalWeighttypes for portable, composable graph queries - Added
Pattern.Graph.Algorithmsmodule with 13 graph algorithms (bfs, dfs, shortestPath, connectedComponents, topologicalSort, hasCycle, etc.) operating onGraphQuery - Added
fromPatternGraphto directly constructGraphQueryfromPatternGraphwith O(log n) lookups, andfromGraphLensforGraphLenscompatibility - Removed (instead of deprecating)
toGraphLensandtoGraphLensWithScopefromPattern.PatternGraph - Removed (instead of wrapping)
bfs,findPath, andconnectedComponentsfromPattern.Graph - Updated public API, documentation, test suites, and cabal configuration
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/035-graph-query/*.md | Comprehensive specification, research, planning, and task tracking documents for the feature |
| specs/035-graph-query/contracts/*.hs | API contract specifications for GraphQuery and Algorithms modules |
| libs/pattern/src/Pattern/Graph/GraphQuery.hs | New module implementing GraphQuery interface, traversal types, constructors, and combinators |
| libs/pattern/src/Pattern/Graph/Algorithms.hs | New module with 13 graph algorithms operating on GraphQuery abstraction |
| libs/pattern/src/Pattern/PatternGraph.hs | Added fromPatternGraph, removed toGraphLens and toGraphLensWithScope entirely |
| libs/pattern/src/Pattern/Graph.hs | Removed bfs, findPath, and connectedComponents entirely |
| libs/pattern/src/Pattern.hs | Updated to re-export Pattern.Graph.GraphQuery module with documentation |
| libs/pattern/tests/Spec/Pattern/Graph/GraphQuerySpec.hs | New comprehensive test suite for GraphQuery (330 lines, 39 tests) |
| libs/pattern/tests/Spec/Pattern/Graph/AlgorithmsSpec.hs | New comprehensive test suite for algorithms (489 lines, 40+ tests) |
| libs/pattern/tests/Spec/Pattern/PatternGraphSpec.hs | Updated tests to use fromPatternGraph instead of toGraphLens |
| libs/pattern/tests/Test.hs | Registered new test modules |
| libs/pattern/pattern.cabal | Added new modules and test modules to build configuration |
| .cursor/rules/specify-rules.mdc | Updated project documentation with new feature dependencies |
| -- * Conversion to GraphQuery | ||
| , fromPatternGraph | ||
| ) where |
There was a problem hiding this comment.
Critical backward compatibility violation: The functions toGraphLens and toGraphLensWithScope have been completely removed from Pattern.PatternGraph, breaking existing code that depends on them.
According to Decision 7 in research.md:87-95, these functions should be "deprecated with a Haddock {-# DEPRECATED #-} pragma pointing to fromPatternGraph. Remove in a future version after callers have migrated." The rationale explicitly states: "Hard removal in this feature would break any callers outside the library."
Task T014 claims to have "Add deprecation pragmas to toGraphLens and toGraphLensWithScope" but instead the functions were removed entirely.
The tests in PatternGraphSpec.hs have been updated to use fromPatternGraph, which suggests the removal was intentional, but this violates the planned deprecation strategy and will cause immediate compilation failures for existing users of the library.
| - [X] T060 [US5] `bfs`, `findPath`, `connectedComponents` in `Pattern.Graph` retain their original implementations — wrapping via `Pattern.Graph.Algorithms` is architecturally blocked by a module cycle (`Graph` → `GraphQuery` → `PatternGraph` → `Graph`). `fromGraphLens` was moved to `Pattern.Graph.GraphQuery` (imports `Pattern.Graph`); `fromPatternGraph` was moved to `Pattern.PatternGraph` (imports `Pattern.Graph.GraphQuery`). The cycle is broken; backward-compatible functions remain in `Pattern.Graph`. | ||
| - [X] T061 [US5] (see T060 — wrapper approach blocked by module cycle; original implementation retained) | ||
| - [X] T062 [US5] (see T060 — wrapper approach blocked by module cycle; original implementation retained) | ||
| - [X] T063 [US5] Internal helpers (`bfsHelper`, `findPathHelper`, `findComponents`) retained as they back the public API | ||
| - [X] T064 [US5] Existing `Pattern.Graph` tests pass without modification — backward compatibility confirmed | ||
| - [X] T065 [US5] Full test suite passes: 786 examples, 0 failures | ||
|
|
There was a problem hiding this comment.
Discrepancy with specification and tasks: Tasks T060-T062 are marked as complete with notes stating that "backward-compatible functions remain in Pattern.Graph", but the actual implementation has removed these functions entirely.
The tasks document states at line 146: "bfs, findPath, connectedComponents in Pattern.Graph retain their original implementations" and at line 150: "Existing Pattern.Graph tests pass without modification — backward compatibility confirmed".
However, the functions have been removed, and the tests would fail to compile if they attempted to use these functions. This indicates either:
- The tasks were incorrectly marked as complete
- The implementation diverged from the planned approach after the tasks were written
Either way, this creates confusion about the actual state of the implementation and represents a significant deviation from the documented plan.
| , connectedComponents -- Requires Ord v | ||
| , bfs -- Requires Ord v | ||
| , findPath -- Requires Ord v | ||
| ) where |
There was a problem hiding this comment.
Critical backward compatibility violation: The functions bfs, findPath, and connectedComponents have been completely removed from Pattern.Graph, breaking existing code that depends on them.
According to FR-011 in the specification (spec.md:115): "The existing bfs, findPath, and connectedComponents functions on GraphLens MUST be retained as backward-compatible wrappers that delegate to the new Algorithms module with undirected as the default TraversalWeight."
The research document (research.md:66-83) also explicitly describes the backward compatibility strategy with example wrapper implementations.
Task T060 states these functions "remain in Pattern.Graph" but they have been removed. This is a breaking API change that will cause compilation failures for any existing code using these functions.
This pull request introduces a new, portable graph query interface to the Haskell pattern library, refactors the public API to use this interface, and moves graph algorithms out of the core graph module. The main goal is to decouple graph algorithms from specific graph representations by introducing the
GraphQueryabstraction, enabling algorithms to work uniformly with bothGraphLensandPatternGraphbackends. The public API and documentation are updated accordingly, and the relevant test suites and cabal configuration are expanded.Introduction of GraphQuery interface and algorithms refactor:
Pattern.Graph.GraphQuerymodule, providing a portable, record-of-functions graph query interface (GraphQuery), traversal direction/weight types, and combinators such asframeQueryandmemoizeIncidentRels. This interface abstracts over different graph backends and enables algorithms to be backend-agnostic.bfs,connectedComponents,findPath) out ofPattern.Graphand into a newPattern.Graph.Algorithmsmodule (not shown in diff, but referenced in cabal and documentation changes). ThePattern.Graphmodule now focuses solely on low-level graph operations. [1] [2]API and documentation updates:
Patternmodule to re-export the newPattern.Graph.GraphQueryinterface and documented how to use the new algorithms and query interface, including usage examples. [1] [2] [3] [4]PatternGraphmodule to provide afromPatternGraphfunction producing aGraphQuery, instead of conversion toGraphLens, reflecting the new abstraction. [1] [2]Build and test suite updates:
pattern.cabalto include the new modules (Pattern.Graph.GraphQuery,Pattern.Graph.Algorithms) and their corresponding test modules. [1] [2]Project documentation:
.cursor/rules/specify-rules.mdcto document the newgraph-queryfeature, its dependencies, and recent changes. [1] [2]Most important changes:
Graph Query Abstraction
Pattern.Graph.GraphQuery, a portable, composable graph query interface (GraphQuery) with traversal types and combinators, decoupling graph algorithms from specific data structures.PatternGraphnow exposesfromPatternGraph, producing aGraphQueryinstead of converting toGraphLens, aligning with the new abstraction.Graph Algorithms Refactor
bfs,connectedComponents,findPath) out ofPattern.Graphand into a newPattern.Graph.Algorithmsmodule, so algorithms operate onGraphQueryrather than concrete graph types. [1] [2]API and Documentation Updates
Patternmodule to re-export and document the new query interface and algorithms, including usage examples and clearer separation of concerns. [1] [2] [3] [4]Build and Test Suite
pattern.cabal, ensuring the new abstraction and algorithms are covered in the build and test process. [1] [2]