Skip to content

feat: add /provider test and /provider models slash commands - #272

Merged
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
feat/provider-diagnostics-slash-commands
Aug 14, 2026
Merged

feat: add /provider test and /provider models slash commands#272
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
feat/provider-diagnostics-slash-commands

Conversation

@bkrabach

Copy link
Copy Markdown
Collaborator

Summary

Add two new read-only diagnostic subcommands to close a critical dead end in the provider workflow:

  • /provider test [name] — connectivity check against live mounted providers
  • /provider models [name] — lists models a provider actually offers

When a pin fails with ✗ cannot pin conversation provider 'fable': it is not mounted in this session, users now have an in-session diagnostic path instead of having to leave the session and run shell commands.

Design Decisions

1. Session-mounted provider reality, not disk config

  • Operates on coordinator.get("providers") — the live session instances
  • If mounted set and on-disk config disagree, session reality is what the user needs
  • Mounted instances never closed after testing; only CLI's throwaway instances are

2. Deliberately NOT gated on conversation.provider_pin capability

  • /provider use and auto require it and refuse without it
  • These diagnostics are useful precisely WHEN pinning is unavailable or failing
  • Gating them would remove the diagnostic exactly when it's most needed
  • Tested with pin=None

3. Shared logic at the right seam

  • Existing click commands in commands/provider.py left untouched (disk-config-driven)
  • Genuinely shared mechanic ("call list_models() async-aware") lives once in provider_diagnostics.py
  • Two surfaces cannot drift on what "OK" means

Concurrency & UX

/provider test with no name runs all targets via asyncio.gather, each wrapped in asyncio.wait_for(..., 15s). Wall clock tracks slowest provider (~1.1s cold, ~0.5s warm), not sum. Progress line printed before await.

Fixed markup safety bug: [name] was parsed as Rich markup and silently vanished; switched to <name>.

Testing

  • 29 new tests; 1413 passed / 1 skipped / 13 deselected / 1 xfailed (same baseline pass count)
  • ruff clean; pyright error count identical to baseline (zero in new code)
  • End-to-end verified in live TUI (build 2026.08.14-25e9944):
    • ✓ /help lists all five forms
    • ✓ /provider models anthropic-sonnet — real network, 3 models, aligned
    • ✓ /provider models (no name) — defaults to priority winner, byte-identical output
    • ✓ /provider test anthropic-sonnet — ✓ 0.1s, singular grammar
    • ✓ /provider test (all four) — 4/4 ✓, 3+3+37+31 models across 3 vendors
    • ✓ Concurrency verified — wall clock ≠ sum of individual times
    • ✓ /provider test nope — clean refusal naming what IS mounted, no traceback
    • ✓ Pin regression intact: pin → pinned footer → unpin → priority winner

Files Changed

  • amplifier_app_cli/provider_diagnostics.py (new, 114 lines) — shared primitives
  • amplifier_app_cli/provider_loader.py (+13/-4) — delegates to shared invoke_list_models()
  • amplifier_app_cli/main.py (+171/-4) — slash command dispatch and handlers
  • tests/test_provider_test_and_models_command.py (new, 402 lines, 29 tests)

Out of Scope

Not adding: add, edit, remove, install, manage. They mutate persistent config (different concern). See commit body for full rationale on capability/responsibility separation.

Closes: #267, #268, #269, #271 (extends /provider feature)

Generated with Amplifier

Add two new read-only diagnostic subcommands to the in-session /provider slash
command, closing a diagnostic dead end when provider pins fail.

BEFORE: /provider supported exactly three forms—status, use <name>, auto. When a
pin failed with ✗ cannot pin conversation provider 'fable': it is not mounted in
this session, the user had no in-session way to find out WHY. Bad API key? Wrong
model name? They had to leave the session and run amplifier provider test /
amplifier provider models in a shell.

AFTER: Two new read-only diagnostic forms, callable mid-conversation:
  • /provider test [name]  — connectivity check against the live mounted
    providers. No name tests all, concurrently, bounded at 15s per provider.
  • /provider models [name] — lists models a provider actually offers. No name
    uses the active provider.

FILES
  • amplifier_app_cli/provider_diagnostics.py (new, 114 lines)
    Shared primitives: invoke_list_models(), ProviderTestResult,
    test_provider_connectivity(), format_model_line(). Async-aware;
    15s timeout per provider; single source of truth for connectivity semantics.

  • amplifier_app_cli/provider_loader.py (+13/-4)
    get_provider_models() now delegates to invoke_list_models() rather than
    keeping its own inline implementation. One less independent implementation
    of the same mechanic.

  • amplifier_app_cli/main.py (+171/-4)
    Slash command dispatch wiring, two new handlers (_handle_provider_test,
    _handle_provider_models), usage string generation, error messages.
    Updated /help description; unknown-subcommand error lists all five forms.

  • tests/test_provider_test_and_models_command.py (new, 402 lines, 29 tests)
    Unit and integration coverage for both commands, all error paths.

THREE DESIGN DECISIONS

1. MOUNTED PROVIDER REALITY, NOT DISK CONFIG
   These operate on THIS SESSION'S MOUNTED PROVIDERS, not settings.yaml.
   The existing amplifier provider test/models CLI commands read config from
   disk and instantiate a throwaway provider. The slash versions read
   coordinator.get("providers") — the live mounted instances. That is the
   entire reason to run them mid-conversation instead of in a shell: if the
   mounted set and the on-disk config disagree, the session's reality is what
   the user needs. The session's mounted instances are never closed after
   testing (asserted in tests); only the CLI's throwaway instances are.

2. NOT GATED ON CONVERSATION.PROVIDER_PIN CAPABILITY
   /provider use and /provider auto require pin capability and refuse loudly
   without it. These two are pure read-only diagnostics and are useful
   precisely WHEN pinning is unavailable or failing — gating them would remove
   the diagnostic exactly when it is needed most. Dispatched before the
   capability gate; tested with pin=None.

3. SHARED LOGIC AT THE RIGHT SEAM
   The existing click commands in commands/provider.py were left untouched —
   they are disk-config-driven and their tests mock at that boundary. What is
   genuinely shared is the lower mechanic ("call list_models() on a provider
   object, async-aware") and the definition of "connectivity is OK", both of
   which now live once in provider_diagnostics.py. So the two surfaces cannot
   drift on what "OK" means.

CONCURRENCY & UX
   /provider test with no name runs all targets via asyncio.gather, each
   wrapped in asyncio.wait_for(..., 15s) so one hung provider cannot freeze
   the REPL. Progress line prints before await. Wall clock tracks the slowest
   provider (~1.1s cold, ~0.5s warm), not the sum.

   Markup safety: [name] in usage strings was parsed as Rich markup and
   silently vanished. Switched to <name>, matching /provider use <name> style.

EVIDENCE
  • 29 new tests; 1413 passed / 1 skipped / 13 deselected / 1 xfailed
    Same pass count as baseline (verified re-run against original stash).
    ruff clean; pyright error count identical to baseline (none in new code).

  • Verified end-to-end in live TUI (build 2026.08.14-25e9944), 8 checks:
    ✓ /help lists all five forms
    ✓ /provider models anthropic-sonnet — real network, 3 models, aligned
    ✓ /provider models (no name) — defaults to priority winner, byte-identical
    ✓ /provider test anthropic-sonnet — ✓ 0.1s, singular grammar
    ✓ /provider test (all four) — 4/4 ✓, 3+3+37+31 models across vendors
    ✓ Concurrency confirmed — wall clock ≠ sum(individual)
    ✓ /provider test nope — clean refusal, no traceback
    ✓ Pin regression intact: pin → pinned footer → unpin → priority winner

OUT OF SCOPE
   add, edit, remove, install, manage were NOT added. They mutate persistent
   config, a different concern from "who is answering me right now", and
   mixing them in would blur the line: amplifier provider changes what is
   available tomorrow; /provider changes who is answering now.

Closes: #267, #268, #269, #271 (extends /provider feature)

Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
@bkrabach
Brian Krabach (bkrabach) marked this pull request as ready for review August 14, 2026 20:58
@bkrabach
Brian Krabach (bkrabach) merged commit 583fba6 into main Aug 14, 2026
7 checks passed
@bkrabach
Brian Krabach (bkrabach) deleted the feat/provider-diagnostics-slash-commands branch August 14, 2026 20:58
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