Skip to content

Resolve model engine names through the shared registry (item 5, step 2) - #164

Merged
isayev merged 1 commit into
mainfrom
refactor/model-registry
Aug 13, 2026
Merged

Resolve model engine names through the shared registry (item 5, step 2)#164
isayev merged 1 commit into
mainfrom
refactor/model-registry

Conversation

@isayev

@isayev isayev commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Non-breaking. available_models() returns the same six names in the same order,
and _adapters was private. Suite: 1756 passed, 1 skipped, 70 deselected.

Three lists holding different sets

list contents
ModelFactory._adapters 2 — ANI2X, ANI2XT only
ModelFactory.available_models() 6 — a hand-written literal, derived from nothing
ENGINE_INFO (cli/commands/models.py) 6 — AIMNET, three aimnet variants, two ANI

_adapters held only engines built from a local adapter class. The four
aimnet entries are not adapters at all — they are names resolved through the
aimnet registry. So one list was keyed by "has a local adapter" and two by
"user-facing name", and nothing connected them; they agreed by hand.

This is the finding recorded in #162's body, and it is why step 2 was the larger
half.

What changed

_engines is one registry keyed by user-facing name, with the adapter class as
the value where there is one and None where the name resolves through the
aimnet registry. available_models() derives from it instead of restating it —
adding an engine and forgetting that literal used to leave it working but
undiscoverable.

The import-time assert set(_adapters) == set(BUILTIN_ANI_MODELS) survives and
is now derived from the registry, so the two cannot drift. It moved to module
scope because a comprehension inside a class body cannot see that class's own
attributes — a Python scoping rule, found by it raising NameError on import,
not a style preference.

ENGINE_INFO is deliberately still separate

Moving it into info= belongs with the CLI's changes rather than this factory's.
It is the remaining piece of item 5, and it is named here so the third list is
not mistaken for handled.

Item 5, step 2 -- the two collapsible lists of the three. Non-breaking:
`available_models()` returns the same six names in the same order, and
`_adapters` was private.

Three parallel lists of engine names existed, holding *different* sets:

    ModelFactory._adapters          2 -- ANI2X, ANI2XT only
    ModelFactory.available_models() 6 -- a hand-written literal, derived from nothing
    ENGINE_INFO (cli/commands)      6 -- AIMNET, three aimnet variants, two ANI

`_adapters` held only the engines built from a local adapter class; the four
aimnet entries are names resolved through the aimnet registry, not adapters. So
one list was keyed by "has a local adapter" and two by "user-facing name", and
nothing connected them.

`_engines` is now the one registry, keyed by user-facing name, with the adapter
class as the value where there is one and `None` where the name is resolved
through the aimnet registry. `available_models()` derives from it rather than
restating it -- adding an engine and forgetting that list used to leave it
working but undiscoverable.

The import-time `assert set(_adapters) == set(BUILTIN_ANI_MODELS)` survives, now
derived from the registry so the two cannot drift. It moved to module scope
because a comprehension inside a class body cannot see that class's own
attributes -- a scoping rule, not a style choice.

`ENGINE_INFO` is deliberately still separate. Moving it into `info=` belongs
with the CLI's changes rather than this factory's, and it is the remaining piece
of item 5. Naming it here so the third list is not mistaken for handled.

Suite: 1756 passed, 1 skipped, 70 deselected.
@isayev
isayev merged commit 80ea962 into main Aug 13, 2026
8 checks passed
@isayev
isayev deleted the refactor/model-registry branch August 13, 2026 21:36
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.

1 participant