Open
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 28 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces an entity graph feature that enables querying and visualizing hierarchical relationship graphs around specific ontology classes. It adds new endpoints, schema models, and a BFS-based graph traversal algorithm that discovers ancestor and descendant nodes with configurable depth limits and optional cross-link exploration. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ProjectAPI as Project<br/>API Endpoint
participant Helper as _ensure_ontology<br/>_loaded
participant Service as OntologyService<br/>(build_entity_graph)
participant RDF as RDF Graph<br/>Traversal
participant Response as EntityGraphResponse
Client->>ProjectAPI: GET /{project_id}/ontology/classes/{class_iri}/graph
ProjectAPI->>ProjectAPI: Resolve branch (param or default)
ProjectAPI->>Helper: Ensure ontology loaded for branch
Helper->>Helper: Load/cache ontology if needed
ProjectAPI->>Service: build_entity_graph(class_iri, ancestors_depth, descendants_depth, ...)
Service->>RDF: Validate focus class exists
alt Class not found
RDF-->>Service: Return None
Service-->>ProjectAPI: None
ProjectAPI-->>Client: HTTP 404 "Class not found"
else Class found
Service->>RDF: BFS traverse ancestors (up to depth)
RDF-->>Service: Ancestor nodes/edges
Service->>RDF: BFS traverse descendants (up to depth)
RDF-->>Service: Descendant nodes/edges
opt include_see_also
Service->>RDF: Discover seeAlso cross-links
RDF-->>Service: Related nodes
Service->>RDF: Expand ancestor lineage for seeAlso nodes
end
Service->>Service: Build GraphNode list with metadata
Service->>Service: Build GraphEdge list
Service->>Response: Construct EntityGraphResponse
Response-->>Service: Serialized response
Service-->>ProjectAPI: EntityGraphResponse
ProjectAPI-->>Client: HTTP 200 + JSON graph
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested labels
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 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 |
damienriehl
force-pushed
the
entity-graph-endpoint
branch
2 times, most recently
from
c7fe862 to
b37648d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
ontokit/schemas/graph.py (2)
22-29: Consider usingLiteraltype foredge_type.Similar to
node_type, theedge_typefield has defined values (subClassOf,equivalentClass,disjointWith,seeAlso). ALiteraltype would provide better validation:+EdgeType = Literal["subClassOf", "equivalentClass", "disjointWith", "seeAlso"] + class GraphEdge(BaseModel): """An edge in the entity graph.""" id: str source: str target: str - edge_type: str + edge_type: EdgeType label: str | None = None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ontokit/schemas/graph.py` around lines 22 - 29, The GraphEdge.model currently types edge_type as a plain string; change it to a Literal of the allowed values to enforce validation: update the GraphEdge class's edge_type annotation to use Literal['subClassOf','equivalentClass','disjointWith','seeAlso'] and add the necessary import (typing.Literal or typing_extensions.Literal) at the top of the module so Pydantic will validate those exact values.
8-19: Consider usingLiteraltype fornode_typeto enforce valid values.Per PR objectives,
node_typehas defined values:focus,root,class,individual,property,external,unexplored. Using aLiteraltype provides validation and better documentation:+from typing import Literal + +NodeType = Literal["focus", "root", "class", "individual", "property", "external", "unexplored"] + class GraphNode(BaseModel): """A node in the entity graph.""" id: str label: str iri: str definition: str | None = None is_focus: bool = False is_root: bool = False depth: int = 0 - node_type: str = "class" + node_type: NodeType = "class" child_count: int | None = None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ontokit/schemas/graph.py` around lines 8 - 19, The node_type field should be narrowed to an explicit Literal to enforce allowed values: change GraphNode.node_type's annotation from str to Literal["focus","root","class","individual","property","external","unexplored"] (keeping the default "class"), and add the necessary import for Literal (from typing or typing_extensions if needed) at the top of the file so Pydantic validation and type checkers will enforce and document the valid node types; update any related usages/tests expecting a plain str if necessary.ontokit/api/routes/classes.py (1)
138-138: Consider wrappinginclude_see_alsoinQuery()for consistency.Other query parameters use
Query(default=...)for explicit declaration, butinclude_see_alsouses a bare default value. While functionally equivalent, wrapping it maintains consistency:- include_see_also: bool = True, + include_see_also: bool = Query(default=True),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ontokit/api/routes/classes.py` at line 138, The include_see_also parameter is using a bare default True while other query params use Query(...); update the route handler signature to wrap include_see_also with Query(default=True) (i.e., change include_see_also: bool = True to include_see_also: bool = Query(default=True)) and ensure Query is imported from fastapi in ontokit/api/routes/classes.py so the parameter declaration is consistent with the other query parameters.ontokit/services/ontology.py (1)
380-387: Consider extractingEXTERNAL_NAMESPACESas a module-level constant.This tuple is defined inside the method but doesn't depend on instance state. Moving it to module level improves readability and avoids recreating the tuple on each call:
+# Namespaces considered external for graph node classification +EXTERNAL_NAMESPACES = ( + "http://www.w3.org/2000/01/rdf-schema#", + "http://www.w3.org/2002/07/owl#", + "http://xmlns.com/foaf/0.1/", + "http://purl.org/dc/elements/1.1/", + "http://purl.org/dc/terms/", + "http://www.w3.org/2004/02/skos/core#", +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ontokit/services/ontology.py` around lines 380 - 387, Extract the EXTERNAL_NAMESPACES tuple out of the method and declare it as a module-level constant (uppercase) at the top of ontokit/services/ontology.py so it is created once; remove the local definition inside the method and ensure all usages in that method reference the module-level EXTERNAL_NAMESPACES constant instead of a local variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ontokit/services/ontology.py`:
- Around line 354-364: The return annotation EntityGraphResponse on
build_entity_graph is undefined at function-definition time; fix by importing
the type at module scope (e.g., add from ontokit.schemas.graph import
EntityGraphResponse near the top of the module) so the annotation resolves, and
remove or keep the redundant local import inside build_entity_graph only if
needed for runtime types like GraphNode/GraphEdge; alternatively you may add
from __future__ import annotations at the top to defer annotation evaluation if
you prefer that approach.
- Around line 413-427: Remove the unused depth parameter from the _classify_node
function signature and body (change def _classify_node(uri: URIRef, is_focus:
bool): ...), and update every call site that currently passes a depth argument
to only pass the uri and is_focus parameters; specifically locate and fix the
call that invoked _classify_node with three args (the call mentioned in the
review) to supply only the two required arguments. Ensure any references in
higher-order uses (e.g., map/filter or callbacks) match the new two-argument
signature and run tests/type checks to confirm no remaining usages of the
removed parameter.
- Around line 540-566: The seeAlso outgoing and incoming loops incorrectly use
break when either the item is not a URIRef or the sa_count limit is reached;
change the logic so non-URIRef items simply continue (e.g., check isinstance and
continue) and only exit the loop when sa_count >= max_see_also_per_node
(separate check before processing or break after incrementing); update both
loops that iterate graph.objects(node_uri, RDFS.seeAlso) and
graph.subjects(RDFS.seeAlso, node_uri) and ensure you still call _make_node,
append to see_also_nodes, and call _add_edge exactly as before when items are
valid.
---
Nitpick comments:
In `@ontokit/api/routes/classes.py`:
- Line 138: The include_see_also parameter is using a bare default True while
other query params use Query(...); update the route handler signature to wrap
include_see_also with Query(default=True) (i.e., change include_see_also: bool =
True to include_see_also: bool = Query(default=True)) and ensure Query is
imported from fastapi in ontokit/api/routes/classes.py so the parameter
declaration is consistent with the other query parameters.
In `@ontokit/schemas/graph.py`:
- Around line 22-29: The GraphEdge.model currently types edge_type as a plain
string; change it to a Literal of the allowed values to enforce validation:
update the GraphEdge class's edge_type annotation to use
Literal['subClassOf','equivalentClass','disjointWith','seeAlso'] and add the
necessary import (typing.Literal or typing_extensions.Literal) at the top of the
module so Pydantic will validate those exact values.
- Around line 8-19: The node_type field should be narrowed to an explicit
Literal to enforce allowed values: change GraphNode.node_type's annotation from
str to
Literal["focus","root","class","individual","property","external","unexplored"]
(keeping the default "class"), and add the necessary import for Literal (from
typing or typing_extensions if needed) at the top of the file so Pydantic
validation and type checkers will enforce and document the valid node types;
update any related usages/tests expecting a plain str if necessary.
In `@ontokit/services/ontology.py`:
- Around line 380-387: Extract the EXTERNAL_NAMESPACES tuple out of the method
and declare it as a module-level constant (uppercase) at the top of
ontokit/services/ontology.py so it is created once; remove the local definition
inside the method and ensure all usages in that method reference the
module-level EXTERNAL_NAMESPACES constant instead of a local variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1a7daa8-6c2e-45c9-b31c-075e27fc2657
📒 Files selected for processing (4)
ontokit/api/routes/classes.pyontokit/api/routes/projects.pyontokit/schemas/graph.pyontokit/services/ontology.py
Comment on lines
+413
to
+427
| def _classify_node(uri: URIRef, is_focus: bool, depth: int) -> str: | ||
| iri = str(uri) | ||
| if is_focus: | ||
| return "focus" | ||
| if _is_external(iri): | ||
| return "external" | ||
| # Check if individual (instance, not a class) | ||
| if (uri, RDF.type, OWL.Class) not in graph: | ||
| for rdf_type in graph.objects(uri, RDF.type): | ||
| if rdf_type in (OWL.ObjectProperty, OWL.DatatypeProperty, OWL.AnnotationProperty): | ||
| return "property" | ||
| return "individual" | ||
| if _is_root_class(uri): | ||
| return "root" | ||
| return "class" |
There was a problem hiding this comment.
Remove unused depth parameter from _classify_node (ARG001).
The pipeline reports ARG001: Unused function argument 'depth'. The parameter is declared but never referenced in the function body:
- def _classify_node(uri: URIRef, is_focus: bool, depth: int) -> str:
+ def _classify_node(uri: URIRef, is_focus: bool) -> str:Also update the call site on line 461:
- node_type=_classify_node(uri, is_focus, depth),
+ node_type=_classify_node(uri, is_focus),🧰 Tools
🪛 GitHub Actions: Distribution
[error] 413-413: ruff check (ARG001) Unused function argument: depth in _classify_node(uri: URIRef, is_focus: bool, depth: int).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ontokit/services/ontology.py` around lines 413 - 427, Remove the unused depth
parameter from the _classify_node function signature and body (change def
_classify_node(uri: URIRef, is_focus: bool): ...), and update every call site
that currently passes a depth argument to only pass the uri and is_focus
parameters; specifically locate and fix the call that invoked _classify_node
with three args (the call mentioned in the review) to supply only the two
required arguments. Ensure any references in higher-order uses (e.g., map/filter
or callbacks) match the new two-argument signature and run tests/type checks to
confirm no remaining usages of the removed parameter.
8dbb271 to
d740837
Compare
Add GET /projects/{id}/ontology/classes/{iri}/graph endpoint that builds
a multi-hop entity graph via BFS traversal. Returns nodes and edges for
visualization with lineage-based node types (focus, root, class, etc.).
Configurable: ancestors_depth, descendants_depth, max_nodes, include_see_also.
Truncation detection when node count exceeds max_nodes.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The entity graph BFS only checked outgoing rdfs:seeAlso (graph.objects), missing incoming connections (graph.subjects). For example, "Proceeding Closed / Disposed seeAlso Motion to Dismiss" was invisible because only MTD's outgoing seeAlso was checked, not what points TO MTD. Now checks both directions, surfacing all cross-branch root ancestors (e.g., "Service", "Status") that connect via seeAlso to visited nodes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
JohnRDOrazio
force-pushed
the
entity-graph-endpoint
branch
from
d740837 to
0ebdbfa
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET /projects/{id}/ontology/classes/{iri}/graph— server-side BFS graph traversalEntityGraphResponsewithGraphNodeandGraphEdgemodelsancestors_depth,descendants_depth,max_nodes,include_see_alsoCompanion PR
Frontend: CatholicOS/ontokit-web#88
Test plan
GET /projects/{id}/ontology/classes/{iri}/graphreturns nodes + edges for a valid class IRImax_nodesinclude_see_also=falseexcludes seeAlso cross-links🤖 Generated with Claude Code
Summary by CodeRabbit