Skip to content

Stop traverse early when max_nodes is reached - #53

Open
gkennos wants to merge 2 commits into
mainfrom
fix/traverse-max-nodes-fanout
Open

gkennos wants to merge 2 commits into
mainfrom
fix/traverse-max-nodes-fanout

Conversation

@gkennos

@gkennos gkennos commented Nov 6, 2026

Copy link
Copy Markdown
Member

Summary

This is to fix issue described in #52

Checklist

  • Applied exactly one label (breaking, feature, fix, dependencies, or chore)
  • Tests pass locally (uv run pytest -q)
  • Lint passes (uv run ruff check .)

@gkennos gkennos added the fix Bug fix, backwards-compatible. PATCH: x.y.z+1 label Nov 6, 2026
@gkennos
gkennos requested a balanced review from Copilot November 6, 2026 10:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Stopping at the node budget does not explicitly close the underlying streaming SQLAlchemy result, potentially leaving its cursor open while the supplied session remains active.

1 open issue
What changed in this PR

This PR addresses #52 by stopping traversal when the node budget is discovered, reducing unnecessary edge materialization for high-fan-out concepts.

Changes:

  • Track discovered nodes and stop edge consumption as soon as max_nodes is reached.
  • Add opt-in batched fetching to KnowledgeGraph.iter_edges.
  • Test traversal limits, edge closure over returned nodes, and early iterator termination.
File Description
tests/​test_traverse.py Adds regression tests for capped traversal and streaming options.
src/​omop_graph/​graph/​traverse.py Enforces the discovery budget during expansion and closes edge generators.
src/​omop_graph/​graph/​kg.py Adds optional batched result fetching for edge iteration.

Review effort: Balanced

Comment thread src/omop_graph/graph/kg.py Resolved
@gkennos
gkennos requested a review from nicoloesch November 6, 2026 10:52

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug fix, backwards-compatible. PATCH: x.y.z+1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants