[spark-compete] fix(cli): name supported providers in call_llm_doctor fallback error#736
Conversation
QA write-up — for reviewer eyesTL;DR. Beforeif provider == "ollama":
return ollama_chat_completion(target, prompt)
raise SystemExit(f"Spark Doctor cannot directly call provider `{provider}` yet.")The operator has no way to know which providers Spark Doctor can call directly without leaving the shell. Afterif provider == "ollama":
return ollama_chat_completion(target, prompt)
raise SystemExit(
f"Spark Doctor cannot directly call provider `{provider}` yet. "
"Supported providers: anthropic (via claude_oauth), codex, huggingface, kimi, "
"minimax, ollama, openai, openrouter, zai. "
"Run `spark providers list` to see configured paths, or `spark setup` to switch."
)The 9 providers come directly from the if-chain immediately above the fallback (L10588-L10597). The recovery commands are the same two SmokeScope
Disjoint with sibling PRs
|
|
R24 cleanup: this source PR has received R24 leaderboard credit and is represented in the shipped/adopted release path by spark-cli#754. Thanks for the contribution; closing this PR to keep the review queue clean. |
{
"schema": "spark-compete-hotfix-v1",
"event": "spark-compete-first-event",
"submission_mode": "public_repo_pr",
"submission_target_url": "#736",
"team": {
"name": "SparkThisUp",
"members": ["ValHallaBuilder", "Baz707", "DanFireDash"],
"github_accounts": ["4gjnbzb4zf-sudo"],
"llm_device_holder": "ValHallaBuilder",
"device_holder_github": "4gjnbzb4zf-sudo"
},
"target_repo": {
"id": "vibeforge1111/spark-cli",
"source": "https://github.com/vibeforge1111/spark-cli",
"owner_surface": "spark-cli"
},
"issue": {
"type": "usage_friction",
"severity": "low",
"title": "call_llm_doctor fallback 'cannot directly call provider' omits the supported provider set",
"actual_behavior": "src/spark_cli/cli.py:10598 raises SystemExit(f"Spark Doctor cannot directly call provider
{provider}yet.") when an unexpected provider string falls through the if-chain in call_llm_doctor (the earlyunsupportedbranch at L10583 already lists 9 providers, but the final fallback at L10598 names only the offending value). The operator runningspark doctor llmafter a setup drift -- e.g. a stale config still pointing at a provider id that the if-chain no longer routes -- sees only the typed-but-routed-nowhere name and has no pointer back to the legal set or to the recovery commands. Compared with the siblingunsupportedbranch on L10583-L10585 (which already names 9 providers), the fallback is the inconsistent sibling.","expected_behavior": "The fallback message names the same supported provider set the if-chain accepts (anthropic, codex, huggingface, kimi, minimax, ollama, openai, openrouter, zai) and points to the two recovery commands (
spark providers listandspark setup). The behavior on a recognised provider is byte-identical -- the if-chain returns before the fallback fires.","repro_steps": [
"gh pr checkout ",
"PYTHONPATH=src python -m unittest tests.test_cli.SparkCliTests.test_call_llm_doctor_unsupported_provider_names_supported_set -> ok (a target dict with provider='experimental-xyz' now raises SystemExit naming all 9 supported providers and the two recovery commands).",
"Manual: PYTHONPATH=src python -c "from spark_cli.cli import call_llm_doctor; call_llm_doctor({'provider':'experimental-xyz','auth_mode':'api'}, 'p')" -> SystemExit message includes 'anthropic, codex, huggingface, kimi, minimax, ollama, openai, openrouter, zai' and 'spark providers list' + 'spark setup'.",
"Pure-hit path unchanged: any of the 9 supported provider ids still routes through the existing if-chain branches without ever reaching the fallback (verified by inspection of L10588-10597)."
],
"affected_workflow": "spark doctor llm -- when the operator's saved provider config has drifted to a name the if-chain no longer routes, the actionable-error fallback now tells them which 9 providers Spark Doctor can directly call and which two commands repair the config."
},
"evidence": {
"safe_links_only": true,
"before_after_proof": "One SystemExit site in call_llm_doctor at src/spark_cli/cli.py:10598. Before: raise SystemExit(f"Spark Doctor cannot directly call provider
{provider}yet."). After: raise SystemExit names the same 9 providers the if-chain at L10588-L10597 accepts, plusspark providers listandspark setupas recovery commands. Diff: 1 file, 6 insertions, 1 deletion in cli.py, plus 27 insertions in tests/test_cli.py for a regression test pinning the new behavior. The 9 supported providers are listed in alphabetical order (anthropic, codex, huggingface, kimi, minimax, ollama, openai, openrouter, zai) to match the siblingunsupportedbranch at L10583-L10585 which already names them.","links": ["https://github.com//pull/736"],
"forbidden": ["pdf","zip","exe","unknown downloads","shortened links","archives","binaries","tokens","browser cookies","wallet material","raw logs","raw conversations","raw memory","raw patches","private repo maps","private scoring details"]
},
"proposed_fix": {
"approach": "Extend the SystemExit message string at src/spark_cli/cli.py:10598 to name the 9 supported providers the if-chain at L10588-L10597 accepts, and to name the two recovery commands (
spark providers list,spark setup). No new helper, no behavior change on the pure-hit path -- a recognised provider still routes through the existing if-chain branches before the fallback can fire. The list is hardcoded inline (no new module-level constant) to keep the change additive and the diff a single-message edit.","files_expected": ["src/spark_cli/cli.py", "tests/test_cli.py"],
"tests_or_smoke": "PYTHONPATH=src python -m unittest tests.test_cli.SparkCliTests.test_call_llm_doctor_unsupported_provider_names_supported_set -> ok. The new test asserts the SystemExit message names the offending provider, all 9 supported providers, and both recovery commands. Pre-existing tests in tests.test_cli that exercise the doctor LLM path are unaffected (the if-chain return paths are byte-identical)."
},
"pr": {
"branch": "sentinel/actionable-error/call-llm-doctor-supported-listing",
"title_prefix": "[spark-compete]",
"author_github": "4gjnbzb4zf-sudo",
"body_must_include": ["packet","team","pr_author","repo","actual_behavior","expected_behavior","repro_steps","before_after_proof","tests_or_smoke","duplicate_notes","risk_notes","review_claim"],
"url": "#736"
},
"review_claim": {
"impact_claim": "low",
"evidence_types": ["redacted_terminal_excerpt"],
"duplicate_notes": "Pre-flight
gh pr list --repo vibeforge1111/spark-cli --search 'doctor llm provider' --state allreturned doctor-LLM-related PRs that are scope-disjoint from this one. PR #207, #309, #415, #552 all handle HTTPError/URLError on the wire forspark doctor llm(network surface, not the message text). PR #241 saves a partial report when the LLM probe fails. PR #553 adds a User-Agent header to doctor requests. PR #492 sends User-Agent on OpenAI-compatible HTTP calls. PR #487 falls back to os.environ for the provider API key. None of those PRs touch the SystemExit message text at call_llm_doctor's final fallback (L10598). Sibling actionable-error PRs in this series have widened the equivalent message on other surfaces: PR #511 (Unknown installed module), PR #516 (Unknown bundle), PR #518 (setup-wizard Unknown X), PR #529 (Unknown providers/recommend command). This entry extends the same pattern to the doctor-LLM fallback that no other PR touches.","risk_notes": "Local scope: one file changed for the fix, plus one regression test. The if-chain return paths at L10588-L10597 are byte-identical -- the fallback only fires when the if-chain has not returned. The message is plain text, not parsed by any other code path. No new module-level constant, no new import.",
"review_state_requested": "pr_review"
}
}