Skip to content

Commit da610e4

Browse files
committed
Fix nested request reply service feedback
1 parent 57e6c50 commit da610e4

24 files changed

Lines changed: 1055 additions & 221 deletions

AGENTS.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ uv build --wheel --python 3.12 --out-dir "$wheel_dir"
6464
uv venv --python 3.14 "$test_env"
6565
uv pip install --python "$test_env/bin/python" \
6666
"$wheel_dir"/*.whl "pytest>=8" "frozendict>=2.4" "tornado>=6.5" \
67-
trove-classifiers
67+
"polars>=1.32" trove-classifiers
6868
"$test_env/bin/python" -m pytest python/tests -q -m "not wip"
6969
```
7070

@@ -75,6 +75,19 @@ local Linux VM before reporting completion. Use the Ubuntu/OrbStack workflow in
7575
safety is involved. macOS is the normal local gate. Windows remains a
7676
best-effort CI platform, but portable code should still be maintained.
7777

78+
The x86_64 OrbStack VM on Apple Silicon does not expose AVX/AVX2. Standard
79+
Polars warns about the missing CPU features and then segfaults inside
80+
`_polars_runtime` during ordinary `DataFrame` construction. This is a VM
81+
dependency issue, not an hgraph failure. Always install the compatibility
82+
runtime before running the Python suite in that VM:
83+
84+
```sh
85+
uv pip install --python "$test_env/bin/python" "polars[rtcompat]>=1.32"
86+
```
87+
88+
Do not retry the full suite with standard Polars after seeing its missing-CPU-
89+
features warning; switch to `rtcompat` immediately.
90+
7891
GitHub CI runs after a push and is not a substitute for these local gates. The
7992
user monitors post-push CI and will report platform-specific failures. Do not
8093
wait for or monitor CI unless asked.

docs/source/developer_guide/architecture.rst

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ exceptions exist, each declared rather than accidental:
172172
scheduled then splits by kind — the boundary scheduling matrix:
173173

174174
- **Shared-output relays** (adaptor ``from_graph``/``to_graph``, reference
175-
service outputs, service response outputs, context/shared outputs) are
175+
and subscription service outputs, context/shared outputs) are
176176
**rank-correct and same-cycle**: pairs are declared with
177177
``Wiring::add_same_cycle_pair``, which rank-constrains the paired source
178178
*after* every capture; ``Wiring::finish`` re-ranks the whole graph once
@@ -187,9 +187,12 @@ exceptions exist, each declared rather than accidental:
187187
sanctioned **next-cycle** forwarders: the pairing is rank-free (no rank
188188
dependency at all), and the capture forwards to the service source at
189189
``evaluation_time + MIN_TD`` (current time during ``start``). The temporal
190-
break — not a wiring edge — is what allows a client's request to derive
191-
from the service's own response without creating a wiring cycle, exactly
192-
like ``feedback``.
190+
break ensures client mutations never run the implementation in the same
191+
engine cycle. Request/reply responses additionally pass through an explicit
192+
outer-graph ``feedback`` edge before their shared response source is
193+
published. Request/reply clients are therefore excluded from indirect
194+
service ranking, permitting recursive and mutually dependent request/reply
195+
calls without making this a property of nested-graph consumers.
193196

194197
Anything else pointing backward is a wiring bug.
195198

docs/source/developer_guide/parity_matrix.rst

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -303,13 +303,13 @@ model.
303303
request/reply services, and late subscription to an existing value all
304304
execute through the erased C++ service runtime. Matching public C++
305305
tests cover constrained generics, erased specialization identity,
306-
reply-less requests, and sampled late subscriptions. Request/reply
307-
forwarding deliberately uses one request cycle followed by same-cycle
308-
reply publication, so it is one cycle faster than the old Python
309-
executor. Calling a service from inside a compiled ``map_`` or ``mesh_``
310-
child still requires a cross-child service endpoint import and remains
311-
a nested-boundary task; private Python service-builder layouts are not
312-
compatibility targets.
306+
reply-less requests, and sampled late subscriptions. Request/reply uses
307+
the Python timing model: request capture advances one cycle and reply
308+
publication crosses an outer-graph feedback edge before the client sees
309+
it. Compiled ``map_`` and ``mesh_`` children import their outer service
310+
transport inputs through the nested boundary; request/reply feedback is
311+
owned by the outer implementation, not by the child consumer. Private
312+
Python service-builder layouts are not compatibility targets.
313313

314314
The 215 upstream ``ts_tests`` tests were also copied mechanically to a
315315
temporary directory and run against the current bridge under Python 3.12.8.
@@ -529,8 +529,8 @@ Wiring and node-authoring surface
529529
- Full at a graph boundary
530530
- Path-aware and generic multi-interface implementations, template and
531531
erased descriptors, multiple request arguments, reply-less requests,
532-
and reference-counted subscriptions. Service calls inside compiled
533-
keyed children remain the nested-boundary exception described above.
532+
reference-counted subscriptions, Python-compatible request/reply
533+
feedback, and service calls inside compiled keyed children.
534534
* - ``@adaptor`` / service adaptors
535535
- Full (core and supported families)
536536
- Source/sink/duplex and per-client keyed service-adaptor exchange are

docs/source/developer_guide/roadmap.rst

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -554,12 +554,6 @@ The following are intentional unless separately re-opened:
554554
TSS/TSD delta that nets to no change does not tick. Explicit writes are
555555
unaffected and match upstream exactly: a python node returning the same
556556
scalar each evaluation ticks each time, as do repeated TSD entry writes.
557-
- **Lower-latency request/reply** (ruling 2026-07-18): a request/reply
558-
service round-trip completes in one fewer engine cycle than upstream — the
559-
reply is observable at the requester one cycle earlier (hg_cpp yields
560-
``[None, value]`` where upstream yields ``[None, None, value]``). This is a
561-
deliberate latency improvement, not a defect; upstream tests that pin the
562-
exact cycle count therefore see the reply one index earlier.
563557
- Python ``REF`` is an opaque value and does not expose ``.output``.
564558
- ``None`` in CompoundScalar/Bundle construction means an unset field.
565559
- TSB deltas are canonically dense; sparse-bundle delta parity is not required.

docs/source/developer_guide/services.rst

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ source/capture pair** — applied with different payloads:
5353
- **Scheduling matrix** (see *Lifecycle Teardown* in :doc:`architecture` for
5454
the invariant statement):
5555

56-
- **Shared-output relays** (reference outputs, service responses, adaptor
56+
- **Shared-output relays** (reference and subscription outputs, adaptor
5757
``from_graph``/``to_graph``, contexts) are **rank-correct and
5858
same-cycle**: pairs are declared with ``Wiring::add_same_cycle_pair``
5959
(source rank-constrained after every capture); ``Wiring::finish`` re-ranks
@@ -66,8 +66,12 @@ source/capture pair** — applied with different payloads:
6666
**next cycle** by design: the pairing is rank-free (no rank dependency),
6767
and the capture schedules the service source for
6868
``evaluation_time + MIN_TD`` (current time during ``start``). The temporal
69-
break replaces a wiring edge, so a client's request may derive from the
70-
service's own response — the same one-cycle rule as ``feedback``.
69+
request mutation does not run the implementation in the capture cycle.
70+
- **Request/reply responses** cross an explicit feedback source/sink pair in
71+
the graph that owns the implementation, then publish through the ordinary
72+
same-cycle shared-output relay. Request/reply clients are omitted from
73+
indirect service ranking, so recursive request/reply calls are legal. No
74+
nested client or higher-order operator constructs this feedback path.
7175
- **Lifecycle:** the source clears its captured state on ``stop``. A restarted
7276
graph must republish through capture before the source can produce a live
7377
shared output.
@@ -107,7 +111,10 @@ The per-flavour payloads:
107111
before the source emits, so the final request delta is **cumulative**
108112
(``make_request_input_source_node`` / ``make_request_input_capture_node``;
109113
proven by "request/reply source emits cumulative client requests" in
110-
``test_service_wiring.cpp``).
114+
``test_service_wiring.cpp``). The implementation output is captured by an
115+
outer-graph feedback sink and replayed on the following cycle before the
116+
keyed shared response is published. A direct request therefore has Python's
117+
observable sequence ``[None, None, response]``.
111118

112119
Related decision recorded with this layer: real-time wall-clock scheduler
113120
alarms use the normal graph schedule queue — ``NodeScheduler(...,
@@ -467,9 +474,9 @@ second implementation for the same service kind + path throws
467474
**Semantics proven by tests** (``test_service_wiring.cpp``): a reference client
468475
reads the implementation output by reference (no copy); paths keep shared
469476
outputs separate; a subscription client's keys reach the implementation on the
470-
next cycle and the response flows back keyed; request/reply replies are keyed
471-
by the client's request id; two clients' requests reach the implementation as
472-
one cumulative delta.
477+
next cycle and the response flows back keyed; request/reply replies cross the
478+
outer feedback edge and remain keyed by the client's request id; two clients'
479+
requests reach the implementation as one cumulative delta.
473480

474481

475482
How a client expression lowers

docs/source/user_guide/authoring_graphs_cpp.rst

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,7 @@ verb**. How the flavours (and the adaptor kinds of the next section) differ:
853853
* - **Request/reply service**
854854
- ``request_schema`` + ``response_schema``
855855
- ``wire<S>(w, request)``
856-
- its **own** reply (next cycle)
856+
- its **own** reply (after request and response cycle boundaries)
857857
- ``TSD<Int, request_schema>`` keyed by client id in; ``TSD<Int, response_schema>`` keyed by the same id out
858858
* - **Adaptor**
859859
- ``: adaptor::interface`` + ``input_schema`` / ``output_schema``
@@ -1010,13 +1010,16 @@ so its reply is erased:
10101010
static Port<TS<Int>> compose(Wiring &w, Port<TS<Int>> request)
10111011
{
10121012
service::register_request_reply_service<AddOneService, AddOneImplNode>(w);
1013-
return wire<AddOneService>(w, request); // this client's own reply, next cycle
1013+
return wire<AddOneService>(w, request); // this client's own delayed reply
10141014
}
10151015
};
10161016
10171017
Two clients of the same service are two independent request-dictionary
10181018
entries; the implementation sees both requests in one cumulative delta and
1019-
each client receives only its own reply.
1019+
each client receives only its own reply. Request capture advances one engine
1020+
cycle and response publication crosses a second, outer-graph feedback edge;
1021+
the observable sequence for one request is therefore
1022+
``[None, None, response]``, matching Python.
10201023

10211024
Paths and other service mechanics
10221025
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

0 commit comments

Comments
 (0)