Skip to content

Feat preflight check #43

Merged
rosspeili merged 3 commits into
ARPAHLS:mainfrom
AaineeSinha:feat-preflight-check
Jun 9, 2026
Merged

Feat preflight check #43
rosspeili merged 3 commits into
ARPAHLS:mainfrom
AaineeSinha:feat-preflight-check

Conversation

@AaineeSinha

@AaineeSinha AaineeSinha commented Jun 7, 2026

Copy link
Copy Markdown

Closes Issue #36

Summary of the Issue & Changes

1. What the Problem Was

When users configured the framework to use local models via Ollama, the CLI would launch straight into the main application menu without checking if Ollama was actually running or if the requested model was downloaded. This led to abrupt runtime crashes or frozen states mid-session if:

  • The Ollama local server wasn't running (ollama serve was omitted).
  • The user specified a model tag in rooms.settings.yaml that they hadn't pulled locally yet.

2. What I Changed (The Solution)

I implemented a robust Preflight Verification Loop that catches these environment misconfigurations early, right after settings load and before any core agent orchestrations run.

Here is the exact breakdown of the architectural changes:

  • Isolated Preflight Logic (rooms/ollama_preflight.py): Created a lightweight, dependency-free utility module. It uses Python’s built-in urllib to ping {settings.ollama.base_url}/api/tags. It cleanly parses the JSON payload from Ollama, extracts the local tag names, and safely checks if the model configured under settings.defaults.litellm_model exists.

  • CLI Interception Loop (cli.py): Integrated the check inside cli.py right after settings initialization. If the default model string starts with ollama/, it runs the preflight check. If the server is unreachable or the model tag is missing, it prints a clean warning panel via the Rich library with actionable instructions (e.g., prompt to run ollama pull), allowing the user to seamlessly troubleshoot or choose to bypass.

  • Bypass Flags for CI/Testing (cli.py): Added a new --skip-preflight CLI option next to the existing --config setup. This allows automated tests, CI/CD matrices, or headless runners to skip the network check entirely.

  • Mock Integration Tests (tests/test_cli_settings_smoke.py): Expanded the test suite to safely validate the new preflight behavior. Using unittest.mock, I mocked the standard library network handlers to cleanly simulate an absolute connection timeout, a successful response returning a list of valid tags, and a response indicating a missing tag. All tests execute completely locally with zero external network overhead in the CI environment.

  • Documentation Review (docs/EXAMPLES.md): Appended an advanced CLI execution block to the end of the documentation file to ensure users and developers know exactly how to leverage the new --skip-preflight flag.

@AaineeSinha AaineeSinha changed the title Feat preflight check Feat preflight check #36 Jun 7, 2026
@AaineeSinha AaineeSinha changed the title Feat preflight check #36 Feat preflight check Jun 7, 2026
@rosspeili

Copy link
Copy Markdown
Contributor

Hey @AaineeSinha, the hook point in cli.py and the smoke-test patch are in the right direction.

Before merge we need a few things in the same PR:

  1. rooms/ollama_preflight.py is missing from the branch, cli.py imports it, so python cli.py would crash. Please add the module (urllib → /api/tags, check defaults.litellm_model when it starts with ollama/, Rich warning panel).
  2. --skip-preflight is documented in EXAMPLES but not wired in build_parser() / main(), please implement it.
  3. Tests, add tests/test_ollama_preflight.py with mocked HTTP (unreachable Ollama, missing tag, happy path). The current smoke test only mocks check_ollama_preflight, which hides the missing module in CI.
  4. Rebase on latest main if you can (changelog / deps PRs may have landed).

Once those are in, happy to review again. Thanks!

@AaineeSinha

Copy link
Copy Markdown
Author

Hey @rosspeili , I've implemented the missing ollama_preflight.py module, wired up the --skip-preflight flag, added full HTTP-mocked unit tests, and cleanly rebased onto main. All 36 tests are passing and it's ready for another look!

@rosspeili rosspeili merged commit 57910c0 into ARPAHLS:main Jun 9, 2026
1 check passed
@rosspeili

Copy link
Copy Markdown
Contributor

Thanks for the follow-up @AaineeSinha, this looks good now. The preflight module, --skip-preflight wiring, and mocked unit tests cover what #36 needed, and the Rich panels with clear next steps are exactly what we wanted.

LGTM from my side once CI is green. Nice and clean work, and thanks for sticking with the review rounds!

@rosspeili

Copy link
Copy Markdown
Contributor

Hey @AaineeSinha you should use your real email when commiting so that you are part of the contributing history. You can change your past commits with your real email and they will be retroactively attributed to you. Now there is a generic github example mail.

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