Skip to content

Fix disabled shipped personas fallback#39

Draft
dogxii wants to merge 1 commit into
ARPAHLS:mainfrom
dogxii:fix/empty-shipped-personas
Draft

Fix disabled shipped personas fallback#39
dogxii wants to merge 1 commit into
ARPAHLS:mainfrom
dogxii:fix/empty-shipped-personas

Conversation

@dogxii
Copy link
Copy Markdown

@dogxii dogxii commented May 31, 2026

Summary

  • Respect use_shipped_personas: false when no YAML personas are configured.
  • Add regression coverage for both an explicit empty personas list and a settings file without a personas key.
  • Remove unused imports from the touched settings test module.

Closes #31.

Validation

  • PYTHONPATH=. venv/bin/python -m pytest tests/test_settings.py -q
  • PYTHONPATH=. venv/bin/python -m pytest tests/test_settings.py tests/test_cli_settings_smoke.py tests/test_cli.py -q
  • PYTHONPATH=. venv/bin/python -m pytest tests/ -q
  • venv/bin/python -m flake8 rooms/settings.py tests/test_settings.py --count --select=E9,F63,F7,F82 --show-source --statistics
  • venv/bin/python -m flake8 rooms/settings.py tests/test_settings.py --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics

@rosspeili
Copy link
Copy Markdown
Contributor

Thanks for picking up #31 @dogxii. The core fix is correct, and I especially like the extra test that loads use_shipped_personas: false from a real settings file via load_settings(). That covers the user path better than a model-only test.

Before this is ready to merge, a few things in the same PR please:

  1. Restore test imports, the diff removes pytest, yaml, os, and Path from tests/test_settings.py, but other tests in that file still need them. CI will fail without those imports.
  2. CLI hint, when get_default_personas() returns an empty list, show a short wizard message (e.g. add personas: in rooms.settings.yaml or create a custom agent) instead of only printing an empty “Available Default Personas” section.
  3. get_default_personas() docstring, document the empty-list case when use_shipped_personas: false and no custom personas are set.
  4. Docs, one line in docs/EXAMPLES.md or docs/ARCHITECTURE.md about use_shipped_personas: false requiring YAML personas and/or custom agents.

Also note there’s an overlapping PR (#34) for the same issue, once yours is updated and marked ready, we’ll merge one and close the other since no one claimed this issue, it will be first come first serve. Thanks again! and if you wanna take your time in solving issues, please feel free to comment under them in your next contributions, so issues can be specifically assigned to you, and we avoid overlapping PRs. <3

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.

use_shipped_personas: false with empty personas still loads shipped personas

2 participants