Skip to content

Python RPC: don't send prefix/markers for delegating wrapper types#7424

Merged
knutwannheden merged 1 commit intomainfrom
fix-python-rpc-_receive_space-no_change
Apr 18, 2026
Merged

Python RPC: don't send prefix/markers for delegating wrapper types#7424
knutwannheden merged 1 commit intomainfrom
fix-python-rpc-_receive_space-no_change

Conversation

@knutwannheden
Copy link
Copy Markdown
Contributor

Motivation

Flagship recipe runs (Common static analysis issues) were failing on every Python repository with the following signature propagated back from the Python RPC server:

Expected positions array but got:
RpcObjectData(state=<RpcObjectState.NO_CHANGE: 'NO_CHANGE'>, value_type=None, value=None, ref=None, trace=None)

...python_receiver.py:864 in _receive_space
    comments = q.receive_list_defined(space.comments)
...receive_queue.py:266 in receive_list
    raise RuntimeError(f"Expected positions array but got: {positions_msg}")

Affected Apr 17 runs included 8 finos/legend-juju-* / finos/symphony-bdk-python repos plus Netflix/metaflow.

Root cause

Py.ExpressionStatement and Py.StatementExpression delegate prefix and markers to their wrapped child, so those fields should be serialized via the child's preVisit, not the wrapper's. Three of the four sides already honored this:

  • Java PythonReceiver.preVisit — only reads id for these types.
  • Python PythonRpcSender._visit — only sends id.
  • Python PythonRpcReceiver._visit — attempted to drain them via q.receive(None).

But Java PythonSender.preVisit unconditionally emitted id + prefix + markers for every J. The Python receiver's q.receive(None) consumed only the header message, not the nested Space comments-list and whitespace messages. The queue then desynchronized, and a later receive_list_defined for Space.comments (often under Py.Await.expression as in the stack above) saw a NO_CHANGE where it expected the positions array.

The failure only surfaced for CHANGE (recipe-modified tree), not ADD, because on ADD the receiver can recover via the valueType on the header and instantiate a Space to drain via its codec. CHANGE omits valueType so no codec runs.

Summary

  • PythonSender.preVisit: skip prefix / markers for Py.ExpressionStatement and Py.StatementExpression, matching the receiver contract and the Python sender.
  • python_receiver.py::_visit: simplify the wrapper-type branch to only read id, since the sender no longer emits the extra messages.
  • Add PythonSenderReceiverRoundTripTest reproducing the production signature via a CHANGE round trip of an ExpressionStatement(Await(Identifier)) with a modified prefix — fails without the PythonSender change with the same Expected CHANGE with positions in receiveList error.

Test plan

  • ./gradlew :rewrite-python:test --tests PythonSenderReceiverRoundTripTest — passes with the fix, fails without it (verified by reverting the sender change).
  • ./gradlew :rewrite-python:test — full Java test suite green.
  • uv run pytest tests/ --timeout=60 --ignore=tests/rpc — full Python unit suite (1157 tests) green.

`Py.ExpressionStatement` and `Py.StatementExpression` delegate `prefix`
and `markers` to their wrapped child. The Java receiver and the Python
sender already skip them in `preVisit`, but the Java sender was emitting
them anyway. The Python receiver tried to drain those extra messages
with `q.receive(None)`, which only consumes the header — not the nested
`Space` comments-list and whitespace messages. The queue then
desynchronized, and a later `receive_list_defined` for `Space.comments`
under a nested node (typically the expression under a `Py.Await`) hit a
`NO_CHANGE` where it expected the positions array, producing:

  Expected positions array but got:
  RpcObjectData(state=NO_CHANGE, value=None, ...)

The failure only surfaced for CHANGE (recipe-modified tree), not ADD,
because on ADD the receiver can recover via the `valueType` on the
header. Flagship recipes like `Common static analysis issues` regularly
hit this on Python sources.

Align the Java sender with the existing contract: send only `id` for
`Py.ExpressionStatement` / `Py.StatementExpression`. Simplify the
Python receiver to match. A new round-trip test reproduces the
production signature (fails without the Java sender change).
@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Apr 18, 2026
@knutwannheden knutwannheden merged commit cde569b into main Apr 18, 2026
1 check passed
@knutwannheden knutwannheden deleted the fix-python-rpc-_receive_space-no_change branch April 18, 2026 19:59
@github-project-automation github-project-automation bot moved this from In Progress to Done in OpenRewrite Apr 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant