fix(graph): address blocker + high findings from PR #37 review

[damienriehl]

Summary

Follow-up to #37 addressing the 2 blocker and 4 high-severity findings from peer review. Targets entity-graph-endpoint so fixes land before #37 merges to dev — single migration-free, single PR for review.

Recommended merge order: this PR (#XX) → #37 → dev.

Findings addressed

Sev	ID	Issue	Commit
Blocker	B1	/api/v1/ontologies/{id}/classes/graph route in classes.py was unauthenticated AND non-functional — no verify_project_access dependency, and a fresh OntologyService() per request never loads any graph, so calling it raises ValueError("Graph for project ... not loaded") and 500s. The frontend uses the project route (which IS auth'd + ontology-loaded), so this was dead code.	fdc51b1
Blocker	B2	project.label_preferences was silently ignored. Every other ontology endpoint threads them through; here a project configured for Spanish SKOS labels would get English rdfs:labels in graph but Spanish labels in tree/detail/search — visibly inconsistent UX.	6f32696
High	H1	node_type: str and edge_type: str in schemas are too loose. Replaced with GraphNodeType / GraphEdgeType Literal unions matching the frontend's lib/graph/types.ts. Now Pydantic enforces, OpenAPI generates proper enums for clients, and a typo in the service code fails type-check.	92ca7e8
High	H2	if not isinstance(include_see_also, bool): raise ValueError was unreachable through FastAPI (which coerces query params before the service is called). Java-ism dropped; we trust type hints.	6f32696
High	H3	total_discovered = [0] ... total_discovered[0] += 1 was a Python 2 closure workaround. Replaced with nonlocal total_discovered — Python 3-idiomatic.	6f32696
High	H4	_get_see_also_targets and _get_see_also_referrers were inner closures inside build_entity_graph (a 330-line function with 9 nested closures). Could only be exercised through full BFS path. Extracted to module scope at ontokit/services/entity_graph_helpers.py with 12 new unit tests covering FOLIO restriction encoding, dedup, named-superclass exclusion, class-only filtering, etc.	2e6f739

Detailed rationale per fix

B1 — Delete dead unauthed route

# Before: classes.py had /ontologies/{ontology_id}/classes/graph using
# get_ontology_service() which returns OntologyService() with no graph.
# Calling it 500s. Frontend uses /projects/{id}/ontology/classes/graph
# instead (correctly auth'd via verify_project_access in projects.py).

Verified by grep that no client (frontend or other backend code) referenced the deleted route. Companion test class TestClassesGraphRoute in test_graph_routes.py removed.

B2 — Thread label_preferences through

async def build_entity_graph(
    self, ..., label_preferences: list[str] | None = None,
) -> EntityGraphResponse | None:
    # Derive preferred language from preferences (e.g., "rdfs:label@es" → "es")
    # for definition lookup.
    preferred_lang: str | None = None
    for pref_string in label_preferences or []:
        pref = LabelPreference.parse(pref_string)
        if pref is not None and pref.language:
            preferred_lang = pref.language
            break

    def _get_label(uri):
        return select_preferred_label(graph, uri, label_preferences) or local_name

    def _get_definition(uri):
        # Prefer literals matching preferred_lang; fall back to any.
        for predicate in (SKOS.definition, RDFS.comment):
            fallback = None
            for obj in graph.objects(uri, predicate):
                if not isinstance(obj, RDFLiteral): continue
                if preferred_lang and obj.language == preferred_lang:
                    return str(obj)
                if fallback is None: fallback = str(obj)
            if fallback is not None: return fallback
        return None

In routes/projects.py:

project = await _ensure_ontology_loaded(...)  # was: just await
result = await ontology.build_entity_graph(
    ..., label_preferences=project.label_preferences,
)

H1 — Tighten schemas to Literal

GraphNodeType = Literal["focus", "root", "secondary_root", "class",
                       "individual", "property", "external"]
GraphEdgeType = Literal["subClassOf", "equivalentClass", "disjointWith", "seeAlso"]

class GraphNode(BaseModel):
    node_type: GraphNodeType = "class"

class GraphEdge(BaseModel):
    edge_type: GraphEdgeType

Frontend mirror is lib/graph/types.ts — values stay in sync.

H4 — Extract seeAlso helpers

New ontokit/services/entity_graph_helpers.py:

def get_see_also_targets(graph: Graph, uri: URIRef) -> list[URIRef]: ...
def get_see_also_referrers(graph: Graph, uri: URIRef) -> list[URIRef]: ...

These were 60+ lines of nested closures inside build_entity_graph. Now testable in isolation with tests/unit/test_entity_graph_helpers.py covering:

• Direct rdfs:seeAlso triples
• FOLIO restriction encoding (someValuesFrom / allValuesFrom / hasValue)
• Dedup across direct + restriction encodings
• Named-superclass exclusion (so subClassOf SomeClass doesn't get treated as a restriction)
• Restriction-with-unrelated-property exclusion
• Class-only filtering for referrers

build_entity_graph keeps thin wrapper closures so the BFS body reads the same.

Verification

• All 1,479 existing tests pass; 12 new helper tests added → 1,491 passing total
• mypy clean on all 5 changed source files
• ruff check clean on ontokit/ and tests/
• ruff format applied

uv run pytest tests/unit/                        # 1491 passed
uv run mypy ontokit/services/ontology.py \
            ontokit/services/entity_graph_helpers.py \
            ontokit/schemas/graph.py \
            ontokit/api/routes/projects.py \
            ontokit/api/routes/classes.py        # Success
uv run ruff check ontokit/ tests/                # All checks passed

Test plan

• uv run pytest tests/unit/test_entity_graph_helpers.py -v → 12 cases pass (helpers tested in isolation)
• uv run pytest tests/unit/test_entity_graph.py tests/unit/test_graph_routes.py → 55 cases pass (BFS + route)
• Verify GET /api/v1/ontologies/{id}/classes/graph is no longer in the route table (grep'able)
• On a project with label_preferences=["rdfs:label@es", ...], hit the entity-graph endpoint and confirm node labels are Spanish (matching tree/detail/search)
• OpenAPI schema (/docs) shows node_type and edge_type as enum types, not arbitrary strings

Findings deferred to Discussion threads

The original review also flagged 7 medium and 9 low items. They will be filed as a tracked Discussion on this repo for follow-up.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 606f1d99-38ef-48f5-9ea1-045cefd2c6b7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/pr-37-blockers

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 81.08108% with 14 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
ontokit/services/ontology.py	61.53%	6 Missing and 4 partials ⚠️
ontokit/services/entity_graph_helpers.py	90.24%	0 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!