Conversation
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
drivers/neo4j/driver.go (1)
152-178: Reduce duplication and add result hygiene; verify behavior change for callers.The
db.relationshipTypes()block duplicatesdb.labels()verbatim. Consider extracting a helper to scan a single-column string result intokinds. Additionally, unlikeRun()at line 143, neither result is explicitlyClose()d; whileresult.Next()drains the stream, matching the pattern elsewhere in this file would be more consistent.♻️ Suggested refactor
func (s *driver) FetchKinds(ctx context.Context) (graph.Kinds, error) { var kinds graph.Kinds + scanKinds := func(tx graph.Transaction, query string) error { + result := tx.Raw(query, nil) + defer result.Close() + + if err := result.Error(); err != nil { + return err + } + + for result.Next() { + var kind string + if err := result.Scan(&kind); err != nil { + return err + } + kinds = append(kinds, graph.StringKind(kind)) + } + + return result.Error() + } + if err := s.ReadTransaction(ctx, func(tx graph.Transaction) error { - if result := tx.Raw("CALL db.labels()", nil); result.Error() != nil { - ... - } - - if result := tx.Raw("CALL db.relationshipTypes()", nil); result.Error() != nil { - ... - } - - return nil + if err := scanKinds(tx, "CALL db.labels()"); err != nil { + return err + } + return scanKinds(tx, "CALL db.relationshipTypes()") }); err != nil { return nil, err } return kinds, nil }Important: This PR changes
FetchKinds()to return both node labels and relationship types in a single flatKindsset, wheregraph.StringKindproduces an undifferentiatedstringKindtype. Verify that all callers ofFetchKinds()can handle this merged result without relying on the previous labels-only semantics (e.g., for seeding node-kind registries separately from relationship-kind registries).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@drivers/neo4j/driver.go` around lines 152 - 178, The two blocks in FetchKinds (inside s.ReadTransaction) duplicate logic for scanning single-column string results from "CALL db.labels()" and "CALL db.relationshipTypes()"; extract a helper (e.g., scanSingleColumnStrings(tx graph.Transaction, cypher string) ([]graph.Kind, error) or a helper that returns []string) used by FetchKinds to run the query, iterate result.Next(), Scan into a string, append graph.StringKind(kind) to kinds, and ensure you call result.Close() (or the result's Close-like cleanup) after consuming or on error to match the existing Run() pattern; after changing FetchKinds to return both labels and relationship types flattened into kinds, run a quick audit of all callers of FetchKinds to ensure they accept merged stringKind results and update any caller logic that previously assumed labels-only semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@drivers/neo4j/driver.go`:
- Around line 152-178: The two blocks in FetchKinds (inside s.ReadTransaction)
duplicate logic for scanning single-column string results from "CALL
db.labels()" and "CALL db.relationshipTypes()"; extract a helper (e.g.,
scanSingleColumnStrings(tx graph.Transaction, cypher string) ([]graph.Kind,
error) or a helper that returns []string) used by FetchKinds to run the query,
iterate result.Next(), Scan into a string, append graph.StringKind(kind) to
kinds, and ensure you call result.Close() (or the result's Close-like cleanup)
after consuming or on error to match the existing Run() pattern; after changing
FetchKinds to return both labels and relationship types flattened into kinds,
run a quick audit of all callers of FetchKinds to ensure they accept merged
stringKind results and update any caller logic that previously assumed
labels-only semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aec65ccd-a6cd-4b94-b6d3-123f508988e3
📒 Files selected for processing (1)
drivers/neo4j/driver.go
2 participants
Description
Fixes neo4j driver.FetchKinds() not returning edge kinds
Resolves: BED-8058
Type of Change
Testing
go test -tags manual_integration ./integration/...)Screenshots (if appropriate):
FetchKinds now returns edge kinds as well!
Driver Impact
drivers/pg)drivers/neo4j)Checklist
go.mod/go.sumare up to date if dependencies changedSummary by CodeRabbit