[bug] [python] Propagate config model reconstruction errors in Action deserialize

[alnzng]

Linked issue: #920

Purpose of change

This change wants to improve the debuggability of Action config deserialization. Before, Action.__custom_deserialize catch all errors when it reconstruct python-format config models, so if a real error happen (for example ModuleNotFoundError), the error is hidden and the raw value is stored. User can not know the real reason, and the problem show up later as a confusing error, e.g. pemja.core.PythonException: <class 'AttributeError'>: 'list' object has no attribute 'model_dump'.

Now we replace the broad try/except with a explicit isinstance(value, list | tuple)` check, so the real error can propagate and fail fast, and it is easy to find the root cause. Plain values still pass through as before.

Tests

• Added unit test
• Verified in a test app and checked the real error propagated properly

API

N/A

Documentation

• doc-needed
• doc-not-needed
• doc-included

[weiqingy]

The serializer only wraps BaseModel values into the (module, class, value) shape. A plain list/tuple config value is written as-is and comes back from JSON as a plain list, so this gate can't tell a serialized model apart from a genuine list. For config={"tags": ["a", "b"]} the round-trip would reach import_module("a") and raise, where the old broad-except kept the raw list. The java branch above avoids this by tagging wrapped values with @class/value and testing membership first.

I'm not sure a plain list/tuple config value is actually reachable today, so genuinely a question: do we expect list-valued config entries (stop sequences, tags, tool lists)? If so, would a self-describing marker on the python shape, mirroring the java tagging, let the deserializer match on the marker instead of guessing from type, with a round-trip test to lock it in? If plain lists are out of scope here, feel free to ignore.

[alnzng]

@weiqingy Very good question on plain list/tuple config value!

My first thinking was the type check is enough, because normally the model config comes from our own code. That is why I started with the shape check.

But I checked again and your worry is real, even more than "future". add_action(**config) is public and takes any keyword value, so a user can already pass agent.add_action(name=..., func=..., tags=["a", "b"]) today.

In that case the old shape check will treat ["a", "b"] as a model and call import_module("a"), then it breaks. So it is not only a future assumption, it is already reachable now.

So I agree marker is the better way. When we serialize we tag the value clearly, and when we deserialize we only rebuild the entries that have the marker. A plain list stays a plain list. It also keeps python consistent with java, which already tags the wrapped value with @class/value and checks the marker first. Now both sides use the same way, not two different logics.

For compatibility I think there is no problem:

• The plan is built fresh from the code on every submission and shipped inside the job graph, not restored from checkpoint, so a new deserialize never meets an old-format plan.
• On java side the python config path is a generic passthrough (it writes each value as-is and rebuilds it back to a map), so the marker dict goes through the same as before, no java change needed.

Please take a look again on my new changes, thanks.

[weiqingy]

Thanks for digging into this. Good catch that add_action(**config) already makes a plain list reachable today, so this was live, not just future. The marker approach looks right, and the new preserves_plain_list test locks the contract in.

I also checked the compatibility point you raised: deserializePythonConfig on the java side is a generic recursive passthrough, so it never interprets the old tuple or the new marker. The marker dict round-trips the same as before, no java change needed. Your read holds.

Nothing further from me.

[wenjin272]

Thanks for fixing this @alnzng. LGTM