Skip to content

fix(simulation): a loader is a callable returning the backend class - #3600

Merged
cagataycali merged 1 commit into
strands-labs:mainfrom
cagataycali:register-backend-loader-not-class
Sep 13, 2026
Merged

cagataycali merged 1 commit into
strands-labs:mainfrom
cagataycali:register-backend-loader-not-class

Conversation

@cagataycali

Copy link
Copy Markdown
Member

register_backend(name, loader) takes a zero-arg callable that returns the backend class. A class is itself callable, so passing one satisfied the registry's call and produced an instance where a class was expected: registration was accepted, and the failure surfaced later in create_simulation().

register_backend loader vs class

What

A SimEngine subclass passed as loader is refused at the door, naming the remedy (lambda: MyEngine, which also keeps the import deferred). force=True does not waive it - the mistake is not a name conflict. A class-shaped factory that is not a SimEngine stays accepted: that is a legitimate loader (row 5 above, byte-identical).

docs/api-reference.md spelled the parameter cls, so the documented call was the mistake: the keyword form raised TypeError: unexpected keyword argument 'cls', the positional form registered a class. The row now names loader.

Why the row survived

The reference's signature grader read only the leading code span on a table line. The drifted signature sat second in a cell that opened with list_backends(). It now grades every span on the line - 36 rows instead of 20, and that widening is what caught this one.

Tests

tests/simulation/test_factory.py pins the refusal, that force does not bypass it, that a refused call registers no alias, and three accepted-loader controls. Pre-fix 4 F / 47 P, post-fix 51 P. Gate: tests/simulation/ tests/*.py = 26,974 passed / 259 skipped / 0 failed; ruff + mypy clean.

LOC delta: +121 / -6 (+22 changelog fragment, +46 tests, +32 source, +25 grader).

register_backend(name, loader) takes a zero-arg callable that returns the
backend class. A class is itself callable, so passing one satisfied the call
the registry makes and produced an instance where a class was expected: the
registration was accepted and the failure surfaced later inside
create_simulation() as

    AttributeError: 'MyEngine' object has no attribute '__name__'

which names neither the parameter nor the registration that supplied it.
force=True waived nothing, because the mistake is not a name conflict.

A SimEngine subclass passed as loader is now refused at the door, naming the
remedy (lambda: MyEngine, which also keeps the import deferred). A class-shaped
factory that is not a SimEngine stays accepted - that is a legitimate loader.

docs/api-reference.md spelled the parameter `cls`, so the documented call was
the mistake: the keyword form raised TypeError on 'cls' and the positional form
registered a class. The row now names `loader`. The reference's signature grader
read only the leading code span on a table line, and the drifted signature sat
second in a cell that opened with `list_backends()` - it now grades every span
on the line, which is what caught this row.
@cagataycali
cagataycali enabled auto-merge (squash) September 13, 2026 16:15

@yinsong1986 yinsong1986 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This PR closes a silent-failure hole in the simulation backend registry: register_backend(name, loader) expects a zero-arg callable returning the backend class, and because a class is itself callable, passing a SimEngine subclass directly was accepted at registration and only failed later inside create_simulation() (the registry calls the loader and reads __name__ off the result, so the caller got an AttributeError naming neither the parameter nor the registration that caused it). The fix refuses the class at the door with a TypeError naming the remedy (lambda: MyEngine), deliberately not waivable by force=True since the mistake is not a name conflict, while keeping the documented carve-out for a class-shaped factory that is not a SimEngine. The change also fixes the docs/api-reference.md row that spelled the parameter cls (the documented call was itself the mistake) and widens the reference's signature grader to read every code span on a table line rather than only the leading one -- which is what let the drifted second-position signature escape.

What's good

  • The refusal converts a late, unattributed failure into a loud error with a remedy, and the narrowed input never worked -- so this is not a breaking change to any functioning caller.
  • The new TypeError is added to the Raises: block in the same change (AGENTS.md > "A Raises: block names every refusal the function itself makes").
  • Tests pin the refusal, that force does not bypass it, that a refused call registers no name or alias, and three accepted-loader controls including the deliberate non-SimEngine class-shaped-factory carve-out.
  • The grader widening ships with a raised row floor (20 -> 30) and pins register_backend in the expected set, so the widening itself is graded rather than assumed; CI is green against the whole reference.
  • Changelog fragment follows the <number>-<slug>.md convention; error strings are plain ASCII; scope is disciplined.

@cagataycali
cagataycali merged commit 486f2ad into strands-labs:main Sep 13, 2026
14 checks passed
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.

3 participants