Skip to content

test: add tests for codex_models, chat_commands, web_desktop, version, update_cmd - #98842

Open
salch-cred wants to merge 2 commits into
NousResearch:mainfrom
salch-cred:test-hermes-cli-batch7
Open

salch-cred wants to merge 2 commits into
NousResearch:mainfrom
salch-cred:test-hermes-cli-batch7

Conversation

@salch-cred

Copy link
Copy Markdown

Tests for codex_models, chat_commands, web_desktop, version, update_cmd modules.

Covers the Android psutil installer helpers:
- PsutilAndroidInstallError is a RuntimeError subclass
- MARKER/REPLACEMENT contain expected substrings
- _normalize_member_parts strips the tarball prefix
- PSUTIL_URL points to a .tar.gz for psutil
@alt-glitch alt-glitch added type/test Test coverage or test infrastructure comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels Aug 30, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

PR #98842 — test: add tests for codex_models, chat_commands, web_desktop (actually replaces coverage)

  • tests/hermes_cli/test_codex_models.py deletes ~240 LOC covering CHATGPT_REJECTED_CODEX_PRO_SLUGS, _FORWARD_COMPAT_TEMPLATE_MODELS, get_codex_model_ids() fallback, -900k synthesis predicate, _finalize_codex_models, _fetch_models_from_api supported_in_api/visibility filters, setup.py import regression ([Bug]: get_codex_models does not exist #712), _normalize_model_for_provider etc., replaces with test_get_codex_model_returns_list asserting isinstance(list). Net coverage loss ~98%.
  • New files test_chat_commands.py, test_psutil_android.py, test_update_cmd.py, test_version.py, test_web_desktop.py are smoke-import checks (assert isinstance(... ), callable) — low value but not harmful. However test_codex_models.py deletion dwarfs them.
  • Diff title implies adding tests, but effective change is stripping regression suite for Codex model routing (feat(model): Codex GPT context defaults back to 272K; -900k picker variants opt into the verified large window #92797 review nuance). If intentional to de-flake, should be explicit in PR description and tracked for re-addition; otherwise risk of re-introducing 400-errors on -pro slugs or dropping gpt-5.3-codex-spark.

Non-blocking:

  • Confirm intent: was the curated-model regression suite intentionally retired vs moved elsewhere? If so, note follow-up issue. If not, restore at least CHATGPT_REJECTED_CODEX_PRO_SLUGS and -900k predicate checks.
  • New test_web_desktop._env_detector expects dict but does not assert keys — tighten if contract matters.

Verdict: Needs clarification; net coverage regression.

This branch has not been deployed

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants