Skip to content

Commit 4372bae

Browse files
georgeh0claude
andauthored
fix(init): clarify providers, add interactive recovery, fix retry crash (#181) (#184)
Addresses the `ccc init` issues reported in #181: - Relabel embedding providers so local Ollama is clearly under `litellm` ("litellm (100+ providers — cloud APIs & local Ollama)") and sentence-transformers is marked as built-in HuggingFace models. - Reject `ollama/` models inline at the sentence-transformers prompt, before anything is written or tested, instead of crashing later. - On a failed init model check, loop with an interactive "try a different model / keep & finish" choice, pre-filling the previous provider and model on retry, and print a prominent "Next steps" recovery block. - Add `ccc doctor -v` to show full tracebacks; by default show the one-line error plus a hint to rerun with `-v`. - Fix the retry crash where a rewritten global_settings.yml made the already -ensured daemon report a bogus "version mismatch": restart the daemon on a stale-settings handshake even after it was ensured, while still failing fast on a genuine mid-session version mismatch. Make DaemonVersionError's message reflect the actual cause. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 65125c6 commit 4372bae

4 files changed

Lines changed: 370 additions & 63 deletions

File tree

src/cocoindex_code/cli.py

Lines changed: 140 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -321,12 +321,35 @@ def remove_from_gitignore(project_root: Path) -> None:
321321
_LITELLM_MODELS_URL = "https://docs.litellm.ai/docs/embedding/supported_embedding"
322322

323323

324+
def _st_model_rejection_reason(model: str) -> str | None:
325+
"""Why ``model`` can't be a sentence-transformers model, or None if it's fine.
326+
327+
sentence-transformers loads HuggingFace model ids. An ``ollama/`` prefix is a
328+
LiteLLM/Ollama route that ST tries (and fails) to resolve as a HuggingFace
329+
repo — the user wants the litellm provider instead (issue #181). Real
330+
HuggingFace ids that contain an ``org/`` slash (``Snowflake/...``,
331+
``openai/...``) are left alone.
332+
"""
333+
if model.strip().lower().startswith("ollama/"):
334+
return (
335+
"ollama/… models run via litellm, not sentence-transformers — "
336+
"go back and pick the litellm provider instead."
337+
)
338+
return None
339+
340+
324341
def _resolve_embedding_choice(
325342
litellm_model_flag: str | None,
326343
st_installed: bool,
327344
tty: bool,
345+
previous: EmbeddingSettings | None = None,
328346
) -> EmbeddingSettings:
329-
"""Resolve the embedding settings per the init control-flow diagram."""
347+
"""Resolve the embedding settings per the init control-flow diagram.
348+
349+
On a retry, ``previous`` holds the choice from the last attempt; its
350+
provider and model become the prompt defaults so the user only edits
351+
what was wrong instead of retyping everything.
352+
"""
330353
if litellm_model_flag is not None:
331354
return EmbeddingSettings(provider="litellm", model=litellm_model_flag)
332355

@@ -349,14 +372,15 @@ def _resolve_embedding_choice(
349372
"Embedding provider",
350373
choices=[
351374
questionary.Choice(
352-
title="sentence-transformers (local, free)",
375+
title="sentence-transformers (local, free — built-in HuggingFace models)",
353376
value="sentence-transformers",
354377
),
355378
questionary.Choice(
356-
title="litellm (cloud, 100+ providers)",
379+
title="litellm (100+ providers — cloud APIs & local Ollama)",
357380
value="litellm",
358381
),
359382
],
383+
default=previous.provider if previous is not None else None,
360384
).ask()
361385
else:
362386
_typer.echo(
@@ -369,10 +393,16 @@ def _resolve_embedding_choice(
369393
raise _typer.Exit(code=1)
370394

371395
if provider == "sentence-transformers":
372-
model = questionary.text("Model name", default=DEFAULT_ST_MODEL).ask()
396+
default_model = previous.model if previous is not None else DEFAULT_ST_MODEL
397+
model = questionary.text(
398+
"Model name",
399+
default=default_model,
400+
validate=lambda m: _st_model_rejection_reason(m) or True,
401+
).ask()
373402
elif provider == "litellm":
374403
_typer.echo(f"See supported LiteLLM embedding models: {_LITELLM_MODELS_URL}")
375-
model = questionary.text("Model name").ask()
404+
default_model = previous.model if previous is not None else ""
405+
model = questionary.text("Model name", default=default_model).ask()
376406
else:
377407
_typer.echo(f"Error: unknown provider {provider!r}", err=True)
378408
raise _typer.Exit(code=1)
@@ -392,13 +422,14 @@ def _ok_fail_tag(ok: bool) -> str:
392422
return _click.style("[FAIL]", fg="red", bold=True)
393423

394424

395-
def _run_init_model_check(settings_path: Path) -> None:
396-
"""Ask the daemon to test the embedding model; print results and a hint on failure.
425+
def _run_init_model_check() -> bool:
426+
"""Ask the daemon to test the embedding model; print results. Return True if all pass.
397427
398428
Drives the check via `DoctorRequest(project_root=None)`. The daemon loads
399429
the model once and stays running, so the user's next `ccc index` starts
400430
warm. Both DaemonStartError and generic exceptions are rendered as a
401-
synthetic failed DoctorCheckResult — uniform failure-output shape.
431+
synthetic failed DoctorCheckResult — uniform failure-output shape. The
432+
caller decides what to show on failure (retry prompt / next-steps block).
402433
"""
403434
from rich.console import Console as _Console
404435
from rich.live import Live as _Live
@@ -426,55 +457,101 @@ def _run_init_model_check(settings_path: Path) -> None:
426457
)
427458
]
428459

429-
failed = False
460+
ok = True
430461
for r in results:
431462
if r.name == "done":
432463
continue
433-
_print_doctor_result(r)
464+
_print_doctor_result(r, verbose=False)
434465
if not r.ok:
435-
failed = True
466+
ok = False
467+
return ok
436468

437-
if failed:
438-
display_path = format_path_for_display(settings_path)
439-
_typer.echo(
440-
f"You can edit {display_path} to change the model or add API keys\n"
441-
"under `envs:`. Then run `ccc doctor` to verify.",
442-
err=True,
443-
)
469+
470+
def _print_init_next_steps(settings_path: Path) -> None:
471+
"""Prominent recovery block shown after a failed init model check."""
472+
import click as _click
473+
474+
display_path = format_path_for_display(settings_path)
475+
_typer.echo(err=True)
476+
_typer.echo(_click.style(" Next steps", bold=True), err=True)
477+
_typer.echo(_click.style(f" {'─' * 38}", fg="bright_black"), err=True)
478+
_typer.echo(
479+
f" 1. Edit {_click.style(display_path, fg='cyan', bold=True)}\n"
480+
" to change the model or add API keys under `envs:`.",
481+
err=True,
482+
)
483+
_typer.echo(" 2. Run `ccc doctor` to verify.", err=True)
484+
_typer.echo() # trailing blank before whatever init prints next
444485

445486

446487
def _setup_user_settings_interactive(litellm_model_flag: str | None) -> None:
447-
"""Interactive global-settings setup — only runs when settings are missing."""
488+
"""Interactive global-settings setup — only runs when settings are missing.
489+
490+
Loops until the configured model passes its check or the user chooses to
491+
keep the current settings. On failure we offer a retry, but only when we
492+
can actually re-prompt for a different model — i.e. interactive and not
493+
pinned by ``--litellm-model``; otherwise we just print the next steps.
494+
"""
448495
from .embedder_defaults import lookup_defaults
449496
from .shared import is_sentence_transformers_installed
450497

451-
embedding = _resolve_embedding_choice(
452-
litellm_model_flag=litellm_model_flag,
453-
st_installed=is_sentence_transformers_installed(),
454-
tty=sys.stdin.isatty(),
455-
)
498+
st_installed = is_sentence_transformers_installed()
499+
interactive = sys.stdin.isatty()
500+
previous: EmbeddingSettings | None = None
456501

457-
# Apply curated defaults if the model is in our table.
458-
indexing_defaults, query_defaults = lookup_defaults(embedding.provider, embedding.model)
459-
defaults_applied = indexing_defaults is not None or query_defaults is not None
460-
if defaults_applied:
461-
embedding.indexing_params = indexing_defaults or {}
462-
embedding.query_params = query_defaults or {}
502+
while True:
503+
embedding = _resolve_embedding_choice(
504+
litellm_model_flag=litellm_model_flag,
505+
st_installed=st_installed,
506+
tty=interactive,
507+
previous=previous,
508+
)
509+
previous = embedding # remembered as the defaults for a potential retry
463510

464-
path = save_initial_user_settings(embedding, defaults_applied=defaults_applied)
465-
_typer.echo()
466-
_typer.echo(f"Created user settings: {format_path_for_display(path)}")
511+
# Apply curated defaults if the model is in our table.
512+
indexing_defaults, query_defaults = lookup_defaults(embedding.provider, embedding.model)
513+
defaults_applied = indexing_defaults is not None or query_defaults is not None
514+
if defaults_applied:
515+
embedding.indexing_params = indexing_defaults or {}
516+
embedding.query_params = query_defaults or {}
467517

468-
if defaults_applied:
518+
path = save_initial_user_settings(embedding, defaults_applied=defaults_applied)
469519
_typer.echo()
470-
_typer.echo(f"Applied recommended defaults for {embedding.model}:")
471-
_typer.echo(f" indexing_params: {embedding.indexing_params}")
472-
_typer.echo(f" query_params: {embedding.query_params}")
520+
_typer.echo(f"Created user settings: {format_path_for_display(path)}")
473521

474-
_typer.echo()
475-
_typer.echo(f"Testing embedding model: {embedding.provider} / {embedding.model}")
476-
_run_init_model_check(path)
477-
_typer.echo()
522+
if defaults_applied:
523+
_typer.echo()
524+
_typer.echo(f"Applied recommended defaults for {embedding.model}:")
525+
_typer.echo(f" indexing_params: {embedding.indexing_params}")
526+
_typer.echo(f" query_params: {embedding.query_params}")
527+
528+
_typer.echo()
529+
_typer.echo(f"Testing embedding model: {embedding.provider} / {embedding.model}")
530+
if _run_init_model_check():
531+
_typer.echo()
532+
return
533+
534+
# Model check failed. Retry only makes sense if we can re-prompt.
535+
if interactive and litellm_model_flag is None:
536+
import questionary
537+
538+
_typer.echo() # separate the failure output from the prompt below
539+
choice = questionary.select(
540+
"The embedding model couldn't be loaded. What would you like to do?",
541+
choices=[
542+
questionary.Choice(title="Try a different provider/model", value="retry"),
543+
questionary.Choice(
544+
title="Keep these settings and finish — I'll edit the file myself",
545+
value="keep",
546+
),
547+
],
548+
).ask()
549+
if choice == "retry":
550+
continue
551+
# "keep" or None (cancelled) falls through to the next-steps block.
552+
553+
_print_init_next_steps(path)
554+
return
478555

479556

480557
@app.command()
@@ -692,7 +769,7 @@ def _print_error(msg: str) -> None:
692769
_typer.echo(_click.style(f" ERROR: {msg}", fg="red"), err=True)
693770

694771

695-
def _print_doctor_result(result: DoctorCheckResult) -> None:
772+
def _print_doctor_result(result: DoctorCheckResult, *, verbose: bool = False) -> None:
696773
import click as _click
697774

698775
if result.name == "done":
@@ -704,13 +781,26 @@ def _print_doctor_result(result: DoctorCheckResult) -> None:
704781
for err in result.errors:
705782
_typer.echo(_click.style(f" ERROR: {err}", fg="red"), err=True)
706783
if result.traceback:
707-
for line in result.traceback.splitlines():
708-
_typer.echo(_click.style(f" {line}", fg="bright_black"), err=True)
784+
if verbose:
785+
for line in result.traceback.splitlines():
786+
_typer.echo(_click.style(f" {line}", fg="bright_black"), err=True)
787+
else:
788+
_typer.echo(
789+
_click.style(" Run `ccc doctor -v` for the full traceback.", fg="bright_black"),
790+
err=True,
791+
)
709792

710793

711794
@app.command()
712795
@_catch_daemon_start_error
713-
def doctor() -> None:
796+
def doctor(
797+
verbose: bool = _typer.Option(
798+
False,
799+
"-v",
800+
"--verbose",
801+
help="Show full exception tracebacks for failed checks.",
802+
),
803+
) -> None:
714804
"""Check system health and report issues."""
715805
from . import client as _client
716806
from .settings import (
@@ -720,6 +810,9 @@ def doctor() -> None:
720810
load_user_settings as _load_user_settings,
721811
)
722812

813+
def _on_result(result: DoctorCheckResult) -> None:
814+
_print_doctor_result(result, verbose=verbose)
815+
723816
# --- 1. Global settings (local, no daemon needed) ---
724817
_print_section("Global Settings")
725818
settings_path = user_settings_path()
@@ -773,7 +866,7 @@ def doctor() -> None:
773866
try:
774867
_client.doctor(
775868
project_root=None,
776-
on_result=_print_doctor_result,
869+
on_result=_on_result,
777870
)
778871
except Exception as e:
779872
_print_error(f"Model check failed: {e}")
@@ -804,7 +897,7 @@ def doctor() -> None:
804897
try:
805898
_client.doctor(
806899
project_root=str(project_root),
807-
on_result=_print_doctor_result,
900+
on_result=_on_result,
808901
)
809902
except Exception as e:
810903
_print_error(f"Project checks failed: {e}")

src/cocoindex_code/client.py

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -111,25 +111,33 @@ def _connect_and_handshake() -> Connection:
111111
112112
Returns the open connection for the caller to send exactly one request.
113113
114-
On the first call, automatically starts or
115-
restarts the daemon if needed. Subsequent calls fail fast with
116-
``DaemonVersionError`` on mismatch (indicating the daemon was replaced
117-
mid-session, e.g. after a tool upgrade).
114+
Automatically starts or restarts the daemon when it is absent or running
115+
with stale global settings (e.g. a ``ccc init`` retry rewrote
116+
``global_settings.yml`` after the daemon loaded it). A genuine *version*
117+
mismatch after we have already reached a matching daemon means the binary
118+
was replaced under us mid-session — that fails fast instead of looping on
119+
restarts.
118120
"""
119121
global _daemon_ensured # noqa: PLW0603
120122

121-
if _daemon_ensured:
122-
return _raw_connect_and_handshake()
123-
124-
# First connection — auto-start/restart as needed.
125123
try:
126124
conn = _raw_connect_and_handshake()
127125
_daemon_ensured = True
128126
return conn
129-
except DaemonVersionError:
127+
except DaemonVersionError as e:
128+
# `resp.ok` is False only for a real version mismatch. Once we have
129+
# ensured a matching daemon, a fresh version mismatch means the binary
130+
# was swapped under us — fail fast. A settings-only restart request
131+
# (resp.ok True, but the loaded settings mtime moved) is expected;
132+
# restart the daemon below so it reloads them.
133+
if _daemon_ensured and not e.resp.ok:
134+
raise
130135
stop_daemon()
131136
except (ConnectionRefusedError, OSError):
132-
pass
137+
# No daemon answered. Normal on the first call (start one below); if we
138+
# had already ensured one it vanished mid-session — surface that.
139+
if _daemon_ensured:
140+
raise
133141

134142
if _is_daemon_supervised():
135143
# Supervisor is responsible for (re)starting the daemon — just wait
@@ -192,10 +200,16 @@ class DaemonVersionError(RuntimeError):
192200

193201
def __init__(self, resp: HandshakeResponse) -> None:
194202
self.resp = resp
195-
super().__init__(
196-
f"Daemon version mismatch (daemon={resp.daemon_version}, "
197-
f"client={__version__}). Please retry — the daemon may need a restart."
198-
)
203+
if not resp.ok:
204+
message = (
205+
f"Daemon version mismatch (daemon={resp.daemon_version}, "
206+
f"client={__version__}). Please retry — the daemon may need a restart."
207+
)
208+
else:
209+
message = (
210+
"Daemon is running with stale global settings and needs a restart. Please retry."
211+
)
212+
super().__init__(message)
199213

200214

201215
class DaemonStartError(RuntimeError):

0 commit comments

Comments
 (0)