Skip to content

bug: export() captures __init__ arguments from every class in the chain, so subclasses cannot be rebuilt from a dumped config #272

Description

@toby-coleman

Summary

Summary

ExportMixin wraps __init__ on every class in a component's hierarchy, and each wrapper merges its
arguments into one dictionary on the instance. The exported arguments are therefore the union of
everything passed anywhere along the super().__init__() chain, rather than the arguments given to
the outermost class.

For a component that derives one of its parent's constructor arguments, this has two effects:

  1. export() includes an argument the constructor does not accept, so Process.dump writes a YAML
    file that ProcessBuilder.build cannot load. plugboard process run fails on a config produced
    by Process.dump.
  2. Where a subclass transforms a parent argument instead of deriving a new one, the transformed
    value is exported in place of the original. Each dump and rebuild applies the transformation
    again, and there is no error to show it.

The components in plugboard.library pass their parent's arguments through unchanged, so they are
unaffected. The problem appears as soon as a user subclasses one of them and derives an argument,
which is the pattern the custom component documentation encourages.

Reproduction

"""MRE: ExportMixin captures __init__ arguments from every class in the chain."""

import tempfile
import typing as _t
from pathlib import Path

from plugboard.library import DataWriter
from plugboard.process import LocalProcess, ProcessBuilder
from plugboard.schemas import ComponentArgsDict, ConfigSpec

import msgspec


class DerivedFieldNames(DataWriter):
    """A writer that derives its parent's field_names from its own arguments."""

    def __init__(self, value_field: str, **kwargs: _t.Unpack[ComponentArgsDict]) -> None:
        super().__init__(field_names=["time_stamp", value_field], **kwargs)
        self._value_field = value_field

    async def _convert(self, data):
        return data

    async def _save(self, data) -> None:
        pass


class HalvedChunkSize(DataWriter):
    """A writer that takes chunk_size in records and passes half-records to its parent."""

    def __init__(
        self, field_names: list[str], chunk_size: int, **kwargs: _t.Unpack[ComponentArgsDict]
    ) -> None:
        super().__init__(field_names=field_names, chunk_size=chunk_size * 2, **kwargs)

    async def _convert(self, data):
        return data

    async def _save(self, data) -> None:
        pass


print("1. A derived parent argument appears in the export and blocks reconstruction")
writer = DerivedFieldNames(name="writer", value_field="capacity_mwh")
args = writer.export()["args"]
print("   export()['args'] =", args)
try:
    DerivedFieldNames(**args)
except TypeError as e:
    print("   rebuild raises TypeError:", e)


print("2. The same failure on the dump and rebuild path")
process = LocalProcess(components=[writer], connectors=[], parameters={})
with tempfile.TemporaryDirectory() as tmp:
    path = Path(tmp) / "process.yaml"
    process.dump(path)
    spec = ConfigSpec.model_validate(msgspec.yaml.decode(path.read_bytes()))
    component_args = dict(spec.plugboard.process.args.components[0].args)
    print("   args in the YAML =", component_args)
    try:
        ProcessBuilder.build(spec.plugboard.process)
    except TypeError as e:
        print("   ProcessBuilder.build raises TypeError:", e)


print("3. A transformed parent argument is exported transformed, with no error")
w = HalvedChunkSize(name="w", field_names=["a"], chunk_size=5)
args = w.export()["args"]
print("   built with chunk_size=5, exported as", args["chunk_size"])
for i in range(3):
    w = HalvedChunkSize(**args)
    args = w.export()["args"]
    print(f"   after rebuild {i + 1}, exported as", args["chunk_size"])

Actual output

1. A derived parent argument appears in the export and blocks reconstruction
   export()['args'] = {'name': 'writer', 'value_field': 'capacity_mwh', 'field_names': ['time_stamp', 'capacity_mwh']}
   rebuild raises TypeError: plugboard.library.data_writer.DataWriter.__init__() got multiple values for keyword argument 'field_names'
2. The same failure on the dump and rebuild path
   args in the YAML = {'name': 'writer', 'initial_values': {}, 'parameters': {}, 'constraints': {}, 'resources': None, 'value_field': 'capacity_mwh', 'field_names': ['time_stamp', 'capacity_mwh']}
   ProcessBuilder.build raises TypeError: plugboard.library.data_writer.DataWriter.__init__() got multiple values for keyword argument 'field_names'
3. A transformed parent argument is exported transformed, with no error
   built with chunk_size=5, exported as 10
   after rebuild 1, exported as 20
   after rebuild 2, exported as 40
   after rebuild 3, exported as 80

Expected output

export()['args'] holds the arguments given to the outermost class:

{'name': 'writer', 'value_field': 'capacity_mwh'}

DerivedFieldNames(**args) and ProcessBuilder.build then rebuild the component, and chunk_size
stays at 5 through any number of dump and rebuild cycles.

Cause

ExportMixin.__init_subclass__ wraps __init__ for each subclass:

setattr(cls, "__init__", ExportMixin._save_args_wrapper(cls.__init__, _SAVE_ARGS_INIT_KEY))

Every wrapper in the chain writes to the same instance attribute, merging into what is already
there:

setattr(self, key, {**getattr(self, key, {}), **saved_args, **saved_kwargs})

DerivedFieldNames.__init__ records value_field, then calls super().__init__(), whose wrapper
records field_names on top. The nested call runs last, so a value the subclass computed also
overrides the caller's own value for an argument of the same name, which is what causes the drift in
part 3.

ComponentArgsSpec permits extra fields, so the additional argument travels from the YAML into the
constructor and surfaces as a TypeError from a base class rather than as a configuration error.

Suggested fix

Record the arguments only for the outermost __init__. Either:

  • keep a re-entrancy flag on the instance, so wrappers entered from a nested super().__init__()
    call do not write; or
  • store the saved arguments per class, and have export() read the record belonging to
    type(self).

Both make the export equal to the arguments the caller supplied, which is what reconstruction needs.

Tightening ComponentArgsSpec to reject unknown fields would also turn any remaining mismatch into
a clear configuration error, although it would not address the cause.

Workaround

The subclass can discard the argument its parent recorded:

def __init__(self, value_field: str, **kwargs: _t.Unpack[ComponentArgsDict]) -> None:
    kwargs.pop("field_names", None)
    super().__init__(field_names=["time_stamp", value_field], **kwargs)

This restores the round trip, but it needs adding to every component that derives a parent argument,
and it does nothing for the silent drift in part 3.

Version Information

Plugboard version: 0.7.0
Platform: Linux-6.6.84.1-microsoft-standard-WSL2-x86_64-with-glibc2.39
Python version: 3.12.7

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions