Skip to content

Move shared Rich console to olmo_eval.common.console to break import cycle#160

Merged
finbarrtimbers merged 3 commits into
mainfrom
finbarr/fix-circular-import
Apr 27, 2026
Merged

Move shared Rich console to olmo_eval.common.console to break import cycle#160
finbarrtimbers merged 3 commits into
mainfrom
finbarr/fix-circular-import

Conversation

@finbarrtimbers

Copy link
Copy Markdown
Contributor

Moves the shared Rich console singleton from olmo_eval.cli.utils to a new olmo_eval.common.console module so non-CLI modules don't pull the entire cli package into their import graph.

The cycle this prevents

When a module under olmo_eval.harness.sandbox is imported at module level — as finbarr/prebuild-sandbox (#159) does in scicode/eval.py — the chain is:

from olmo_eval.harness.sandbox import SandboxManager
  → harness/__init__.py: from .presets import (…)
    → presets.py: from olmo_eval.runners.asynq.batching import BatchConfig
      → runners/io/storage.py: from olmo_eval.cli.utils import console
        → triggers cli/__init__.py
          → cli/__init__.py: from olmo_eval.harness.presets import …
            ← harness.presets is half-loaded → ImportError

The fix is to relocate console to a leaf module that imports nothing from cli/harness/runners. cli.utils re-exports it so existing CLI imports are unchanged.

Files

  • src/olmo_eval/common/console.py — new file, just console = Console(force_terminal=True, width=120).
  • src/olmo_eval/cli/utils.py — re-exports from common.console; CLI callers unchanged.
  • src/olmo_eval/runners/io/storage.py — switches to common.console. This is the actual cycle break.
  • src/olmo_eval/runners/common/mixins.py — same swap.
  • src/olmo_eval/common/inspection.py — replaces 7 lazy from olmo_eval.cli.utils import console as shared_console lines inside functions with one top-level import (keeps imports at top of file).

Why standalone

Split out from #159 for independent review. The fix applies cleanly to main and is a no-op for any current import path; it's preemptive infrastructure for #159.

Verification

  • uv run pytest tests/ -q — 1336 passed, 22 skipped.
  • make lint — clean.

…cycle Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@undfined undfined left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good, thanks!

@finbarrtimbers finbarrtimbers merged commit 81c5967 into main Apr 27, 2026
10 checks passed
@finbarrtimbers finbarrtimbers deleted the finbarr/fix-circular-import branch April 27, 2026 20:13
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.

2 participants