Skip to content

refactor(graph): split duckdb_graph module - #190

Merged
limaronaldo merged 1 commit into
mainfrom
chore/split-duckdb-graph-files
Jul 12, 2026
Merged

refactor(graph): split duckdb_graph module#190
limaronaldo merged 1 commit into
mainfrom
chore/split-duckdb-graph-files

Conversation

@limaronaldo

@limaronaldo limaronaldo commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Split src/graph/duckdb_graph.rs into five focused modules: mod.rs, types.rs, lifecycle.rs, queries.rs, traversal.rs, and tests.rs.
  • Kept the public graph::duckdb_graph module surface unchanged via pub mod duckdb_graph in src/graph/mod.rs.
  • Preserved public APIs and behavior by keeping compatibility shims/re-exports where needed.
  • Applied minimal visibility changes (pub(super)) only for internal shared state across the split files.

Verification

  • cargo check -p engram-core --all-features
  • cargo test -p engram-core --all-features graph:: (78 passed, 1680 filtered out)
  • cargo fmt --all
  • cargo clippy -p engram-core --all-features --all-targets
  • cargo public-api -p engram-core --simplified

The public API diff changed vs baseline from 19908 to 20088; this appears to be pre-existing/other drift and is not tied to this refactor.

@huly-for-github

Copy link
Copy Markdown

Connected to Huly®: ENGRA-226

@limaronaldo limaronaldo left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review summary (review-only)

I reviewed PR #190 as-is.

Findings

  • NIT (non-blocking): TemporalGraph internal fields were changed from private to pub(crate) in src/graph/duckdb_graph/types.rs.
    • conn, has_pgq, and sqlite_path are only needed by sibling modules after the split.
    • You can keep internals less exposed by using pub(super) (as discussed in your notes) instead of pub(crate), which also avoids unnecessary API-surface expansion.

Verification completed

  • cargo fmt --all -- --check
  • cargo test -p engram-core --all-features graph:: ✅ (78 passed, 1680 filtered out)
  • PR checks for 190 are all passing in CI (gh pr checks 190 -> 16 passed, 0 failed).

No blocking defects found; this is a clean refactor if the visibility tweak above is acceptable to tighten.

@limaronaldo
limaronaldo merged commit 13e7462 into main Jul 12, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant