Skip to content

fix: facade calls that fail against the current server - #233

Merged
jfrench9 merged 1 commit into
mainfrom
bugfix/facade-review-fixes
Oct 5, 2026
Merged

jfrench9 merged 1 commit into
mainfrom
bugfix/facade-review-fixes

Conversation

@jfrench9

@jfrench9 jfrench9 commented Oct 5, 2026

Copy link
Copy Markdown
Member

Summary

Fixes facade calls that fail against the current server. They came out of a review of the hand-written clients/ layer. The test suite missed them because it answered every call with a mocked 200. The worst one: since server v1.11.10 the operator endpoint answers 202 unless a sync wait is requested. The generated parser turns a 202 into None, so every OperatorClient run and operator_query() call raised "Unexpected response format".

Changes

Fixes

  • Operator: a 202 is read from the raw body and followed to completion, over the stream or by polling /status. OperatorOptions.mode is now sent as ?mode=. An HTTP error raises with its status instead of "Unexpected response format".
  • Cypher query: a queued 202 raises QueuedQueryError or waits on the stream, as documented. One body classifier is now shared by the sync client, the async client and AuthenticatedClients.execute_cypher_query. That last method also read a dict as an object and always returned empty data.
  • Write credentials: writes now send a credential the way reads do. rfs… API keys go in X-API-Key, anything else (a JWT from token_provider) as Authorization: Bearer. Before, every write sent a JWT as X-API-Key and got a 401. The graph, file, table and document clients now honour token_provider.
  • get_graph_info always raised "Graph not found"; fixed.
  • TableClient.list always returned [], because it read a table_type the server doesn't send.
  • monitor_operation hung on a cancelled operation; it now settles. A stream that ends with no terminal event raises.
  • cancel_operation always returned False. It now reads the server's status: "cancelled".
  • Progress now reads the server's progress_percent.
  • AsyncRoboSystemsClients: the async query and operation methods were pass stubs. They are now implemented with the async generated calls.
  • create_report / regenerate_report crashed on every call, because the request model expects date objects and the facade passed strings.
  • FileClient no longer leaks UNSET into FileInfo.

Additions (parity with the TypeScript client)

  • OperatorRunError(message, status, writes, operation_id), raised when a run fails or is cancelled, so a caller can show what the run already wrote. It subclasses Exception, so existing handlers still catch it.
  • create_report(periods=…).
  • list_information_blocks(scenario_id=…). This adds a variable to its .graphql operation and regenerates it.
  • dispose_schedule: memo and reason are now optional, matching the server.

Docs: corrected the retry docstring on handler-raised 429s, the _call_op and GraphClient docstrings, and removed the uncalled _unwrap.

Compatibility

Rides a client minor. The facades are stable tier, but every change is backward compatible: new optional parameters, a new exception that subclasses what was raised before, and a new TableInfo.s3_location field added last with a default. Behaviour an integrator may notice:

  • A credential that does not start with rfs now goes out as Bearer on writes. Every real API key starts with rfs.
  • TableInfo.table_type is always None, because the server has no such field.

Release-note lines:

  • Fixed OperatorClient and operator_query() failing on every run since server v1.11.10.
  • Fixed writes failing with a JWT token_provider; queued Cypher queries; get_graph_info; TableClient.list; cancel_operation; monitoring a cancelled operation; create_report.
  • Implemented the async query and operation clients.
  • Added OperatorRunError, create_report(periods) and list_information_blocks(scenario_id).

Testing

  • New tests/test_facade_wire_shapes.py (30 tests): answers the facades with the server's real status codes and bodies through the real generated parsers. Against the previous HEAD, 25 of the 26 that can run there fail. The 4 monitor tests hang on the old code.
  • Corrected tests that had encoded the old behaviour: the cancel-response body, the Cypher dict body, and the SSE header key.
  • just test-all: 647 passed, 17 skipped. ruff, format and basedpyright are clean.

Follow-ups (not in this PR)

  • OperationClient.monitor_operation's sync timeout is still not enforced, because connect() blocks.
  • A mode="stream" operator call fails when the server streams its 200 directly.

🤖 Generated with Claude Code

Handle the operator and Cypher 202 (queued) responses, route write
credentials the way reads do, and fix the monitor, cancel, progress,
graph-info, table-list and async-facade paths. Add OperatorRunError,
create_report periods, list_information_blocks scenario_id, and make
dispose_schedule memo/reason optional.
@jfrench9
jfrench9 merged commit 05cfe00 into main Oct 5, 2026
4 checks passed
@jfrench9
jfrench9 deleted the bugfix/facade-review-fixes branch October 5, 2026 19:32
jfrench9 added a commit that referenced this pull request Oct 5, 2026
## Summary

`OperationClient.monitor_operation` never applied its `timeout`.
`connect()` blocks until the stream ends, so the wait could last
forever. `GraphClient.materialize` (default 600 s) and `change_tier`
(600 s, documented to raise `TimeoutError`) both pass a timeout
expecting it to work. This was a follow-up from #233.

## Changes

- **Timeout:** a timer closes the stream when `timeout` passes. The read
ends and the call raises `TimeoutError("Operation … timed out after
Ns")`, the same message as the async monitor. The timer is cancelled
once the call returns, so a run that finishes in time is unaffected.
- **Reconnect:** `SSEClient` no longer reopens the stream when `close()`
lands during a reconnect backoff.
- **Type:** `MonitorOptions.timeout` is now `Optional[float]`, in
seconds. This widens the type and removes nothing.

The other #233 follow-up, a `mode="stream"` operator call failing,
turned out not to be a defect. The server only treats `mode=sync`
specially (`routers/graphs/operator/execute.py`). `stream`, `async` and
`auto` all answer `202`, which #233 already follows.

## Compatibility

Rides a client **patch**. Behaviour change: `materialize()` and
`change_tier()` now stop waiting at their timeout (600 s by default)
instead of waiting forever. `change_tier` raises `TimeoutError`, as its
docstring says. `materialize` returns a failed `MaterializationResult`.
The server-side operation keeps running either way.

## Testing

- **New tests:**
  - a silent stream is closed at a 0.1 s timeout and raises;
  - a run that finishes before its timeout returns its result;
  - a close during the reconnect backoff doesn't reconnect.
- **`just test-all`:** 650 passed, 17 skipped. ruff, format and
basedpyright are clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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