Issue #1292: fix: graphql_client raises when a 200 response carries GraphQL errors

[dereck-symmetry]

Description of Change

Problem. A GraphQL server answers HTTP 200 even when an operation fails, and reports the failure in the body's errors. Since #1264 / #1291, that's exactly how the LIF GraphQL API reports a Query Planner failure. graphql_client only called raise_for_status(), so an errors body came back as data. Its one consumer, semantic_search_service, then passed it to the LLM as a tool result ([response_json] if response_json else ..., and a body containing only errors is truthy) instead of raising a ToolError. The README already claimed the client raised on a "GraphQL-error response"; the code didn't.

Solution. graphql_query and graphql_mutation now call a small helper, _raise_on_graphql_errors, after the transport try. It raises GraphQLClientException("GraphQL errors: <messages>") whenever errors is non-empty. The helper sits outside the try so the generic except Exception doesn't re-wrap it as "GraphQL client error". Normal responses, and bodies with an empty errors: [], are returned unchanged.

The partial-success decision (the issue asked for it to be made deliberately). A body with both data and errors also raises. The only caller selects a single root field (person in GRAPH_QL_QUERY_TEMPLATE, updatePerson in the mutation template). So a partial answer can only mean a nested field failed, and returning the rest would let that failure read as data the learner doesn't have, which is the ambiguity #1264 removed. This matches the existing precedent in lif_to_lif_adapter/adapter.py:73, which raises on any "errors" in result. The decision and its reasoning are written down in the component README.

Side effects. The semantic search MCP tools lif_query and lif_mutation now report a GraphQL failure as a ToolError. Both already caught GraphQLClientException and converted it (semantic_search_service/core.py:431-433, :449-451), so they needed no code change. The error text is whatever GraphQL put in message. Since #1291 the query path sends only Query failed: <status>. The mutation path still includes the Query Planner body; that's a separate leak, tracked as #1309.

This doesn't interact with #1308. The partial-answer reason planned there goes in the response extensions, not errors.

How reviewers should test it.

uv run pytest test/components/lif/graphql_client/ -q

To confirm the new tests actually guard the change: revert components/lif/graphql_client/core.py alone and three of them fail.

Related Issues

Closes #1292
Refs #1131

Type of Change

• Bug fix (non-breaking change which fixes an issue)

Project Area(s) Affected

• components/
• test/ or e2e/
• Documentation (docs/, READMEs, ARCHITECTURE.md, CLAUDE.md)

---

Checklist

• commit message follows commit guidelines (see commitlint.config.mjs)
• tests are included (unit and/or integration tests)
• code passes linting checks (uv run ruff check)
• code passes formatting checks (uv run ruff format)
• code passes type checking (uv run ty check)
• pre-commit hooks have been run successfully
• configuration changes: relevant folder README updated

Testing

• Automated tests added/updated

Four new tests in test/components/lif/graphql_client/test_core.py:

Test	Old code	New code
query, errors with data: null	fails (did not raise)	passes
query, errors with partial data	fails (did not raise)	passes
mutation, errors with data: null	fails (did not raise)	passes
query, errors: [] is returned unchanged (guards against over-raising)	passes	passes

The client and consumer suites (graphql_client, semantic_search_service, semantic_search_mcp_server) pass: 29 tests. pre-commit run --files on all four changed files is green, including the full pytest suite.

Additional Notes

Shared brick (#1171). components/lif/graphql_client is packaged only by lif_semantic_search_mcp_server (projects/lif_semantic_search_mcp_server/pyproject.toml:36), and that project's deploy workflow already lists components/lif/graphql_client/** in its paths: (.github/workflows/lif_semantic_search_mcp_server.yml:14). A repo-wide search finds semantic_search_service as the only importer.

Merge order. The only open PR sharing a file with this one is #1148, on CHANGELOG.md. Both add lines under [Unreleased], so whichever merges second keeps both.

🤖 Generated with Claude Code

[bjagg]

Approving. I checked the claims against the code: the consumer converts GraphQLClientException to ToolError on both tools (and its generic except covers anything else), both templates select a single root field, so the partial-success decision holds, semantic_search_service is the only importer, and the paths: coverage is as stated. 29 tests pass; reverting core.py fails exactly the 3 new ones.

I mutation-tested it: skipping the check on either path, returning partial data, over-raising on errors: [], and dropping the messages are all caught. One nit, not blocking: moving _raise_on_graphql_errors back inside the try passes all 29, because the match= patterns are substrings that survive the re-wrap. The outside-the-try placement the description calls out is correct but unguarded. match=r"^GraphQL errors: " on one of the tests would pin it.