Skip to content

Refactor: module-level mutable globals for catalog state and leaked UI components #32

Description

@laceyp99

Where

arena/app.py — module top (MODEL_CATALOG, MODEL_CATALOG_STATUS, OPENROUTER_API_KEY, MODEL_LOOKUP, PROVIDER_CHOICES, PROVIDER_MODELS, DEFAULT_PANEL_MODEL_IDS), _set_model_catalog_state() (7 global statements), and create_demo() (5 more global statements).

Problems

1. Catalog state as 7 parallel globals

Every consumer (_chatbot_label, _provider_for_model, _resolve_model_for_provider, _selectors_interactive, stream_all_models, ...) reads module globals that must be kept mutually consistent by _set_model_catalog_state(). This forces the awkward "wrapper functions around arena/core/models.py private functions" pattern at the top of app.py, including this import style repeated four times:

from arena.core.models import (
    _chatbot_label as _catalog_chatbot_label,
)

A single CatalogState dataclass (catalog, status, ready flag, api_key, lookup, provider index, defaults — with the derived fields computed in __post_init__ or a classmethod) held in one module attribute would collapse _set_model_catalog_state to one assignment, eliminate the aliased private imports, and make tests able to swap state atomically instead of monkeypatching seven names.

2. UI components leaked as globals for tests

global demo
global openrouter_status_banner
global panel_1_model
global panel_1_provider
global send_btn

create_demo() publishes an arbitrary subset of components (only panel 1's, plus the send button) as module globals solely so tests/integration/test_smoke.py can poke at .get_config(). That's an inconsistent, undeclared test API — the names don't exist until create_demo() runs, so importing them earlier raises AttributeError, and panels 2/3 aren't inspectable at all. Prefer returning a small AppHandles object (or dict of named components) from create_demo() and updating the smoke test, then delete the globals.

3. Cross-module reach into private helpers

app.py imports 20+ underscore-prefixed functions from core/, state/, and ui/. If they're the package's real API, drop the underscores; the current convention says "don't use me" about the functions the app is built from.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestpriority:lowUseful cleanup or guardrail work

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions