fix(dx): newcomer ergonomics — tool names, teaching errors, no scaffold leaks

Author: ctrl-gaurav

effgen/cli/_main.py

@@ -711,6 +711,19 @@ def run_agent(self, args):
 
         self.print_header(f"effGen v{__version__} - Running Task")
 
+        # Resolve the model once so the preset and plain paths agree. With no
+        # -m/--model, mirror `quickstart`: prefer a detected cheap cloud model,
+        # else a small local model — and say why, so the choice is never a silent
+        # surprise (a paid cloud call or a multi-GB local download).
+        if args.model:
+            run_model = args.model
+            _preflight_model_hint(self, run_model, provider)
+        else:
+            run_model, _sugg_provider, _sugg_reason = _quickstart_suggest_model()
+            if provider is None and _sugg_provider:
+                provider = _sugg_provider
+            self.print(f"Using model {run_model} ({_sugg_reason}); override with -m/--model.")
+
         agent = None
         try:
             # Load configuration if provided
@@ -728,7 +741,7 @@ def run_agent(self, args):
             # Use preset if specified
             if getattr(args, 'preset', None):
                 from effgen.presets import create_agent as _create_preset_agent
-                model_id = args.model or "Qwen/Qwen2.5-3B-Instruct"
+                model_id = run_model
                 self.print(f"Using preset: {args.preset}")
                 _preset_overrides = {"provider": provider} if provider else {}
                 agent = _create_preset_agent(
@@ -777,7 +790,7 @@ def run_agent(self, args):
                 # Create agent configuration
                 agent_config = AgentConfig(
                     name=args.name or "cli-agent",
-                    model=args.model or "Qwen/Qwen2.5-3B-Instruct",
+                    model=run_model,
                     provider=provider,
                     tools=tools,
                     system_prompt=args.system_prompt or config.get("system_prompt",
@@ -3786,6 +3799,40 @@ def _quickstart_suggest_model() -> tuple[str, str | None, str]:
     return _QUICKSTART_LOCAL_MODEL, None, "no cloud key found — using a small local model"
 
 
+def _preflight_model_hint(cli: "CLIInterface", model_id: str, provider: str | None) -> None:
+    """Surface a clean "did you mean" for an unknown model id, once, up front.
+
+    When the user passes an explicit ``-m`` id that isn't in the local catalog,
+    a high-confidence typo (e.g. ``gpt-5-nanoo``) otherwise only reveals itself
+    mid-run as a provider 404 wall. This checks the local catalog first and, if
+    the id is unknown but has near matches, prints a single tidy suggestion line.
+    It never blocks — the catalog can be stale, so an unknown-but-real new id is
+    allowed through; we only inform.
+    """
+    try:
+        from effgen.models import _catalog
+
+        _bare = model_id.split(":", 1)[-1]
+        # Local / HF-hub ids ("org/model") are resolved by download, not via the
+        # cloud catalog, so a catalog miss there is meaningless — never warn on a
+        # legitimate local model (e.g. meta-llama/Llama-3.2-3B-Instruct). The hint
+        # targets slash-free cloud chat ids, where a typo is the likely cause.
+        if "/" in _bare:
+            return
+        if _catalog.lookup(model_id, provider) is not None:
+            return
+        alts = _catalog.nearest_alternatives(model_id, provider, n=3)
+        if not alts:
+            return
+        names = ", ".join(r.id for r in alts)
+        cli.print(
+            f"Note: '{_bare}' isn't in the local catalog. Did you mean: {names}? "
+            "Proceeding anyway — run 'effgen models refresh' if it's a new id."
+        )
+    except Exception:  # noqa: BLE001 - a hint must never break the run
+        pass
+
+
 def _handle_quickstart_command(args, cli: "CLIInterface") -> int:
     """Handle 'effgen quickstart' / 'effgen tutorial' — a guided first run.
 
@@ -3903,17 +3950,19 @@ def _handle_quickstart_command(args, cli: "CLIInterface") -> int:
             return 1
 
         # --- 4. Show the trace -------------------------------------------
-        if response.execution_trace:
-            cli.print_header("What the agent did")
-            for i, step in enumerate(response.execution_trace, 1):
-                action = step.get("action", step.get("tool", ""))
-                observation = step.get("observation", step.get("output", ""))
-                if action:
-                    cli.print(f"  {i}. used [cyan]{action}[/cyan] → {str(observation)[:80]}"
-                              if cli.console else
-                              f"  {i}. used {action} -> {str(observation)[:80]}")
-            if not any(s.get("action") or s.get("tool") for s in response.execution_trace):
-                cli.print("  (answered directly, no tools needed)")
+        # The agent's execution_trace is a list of event dicts (type/message/
+        # data) — not a ReAct action/observation shape — so render it with the
+        # shared formatter that understands those events. Hand-reading
+        # ``step["action"]`` here used to miss every tool call and wrongly print
+        # "answered directly" even when a tool ran (and tool_calls reported it).
+        cli.print_header("What the agent did")
+        _trace_lines = _progress.execution_trace_lines(response.execution_trace)
+        if _trace_lines:
+            for _style, _text in _trace_lines:
+                if cli.console:
+                    cli.console.print(f"  [{_style}]{_text}[/{_style}]")
+                else:
+                    cli.print(f"  {_text}")
         else:
             cli.print("  (answered directly, no tools needed)")
 

effgen/core/agent.py

@@ -439,7 +439,7 @@ class Agent(AgentGenerationMixin, AgentReActMixin, AgentRuntimeMixin):
 Question: {task}
 {scratchpad}"""
 
-    def __init__(self, config: AgentConfig, session_id: str | None = None):
+    def __init__(self, config: AgentConfig | None = None, session_id: str | None = None):
         """
         Initialize agent.
 
@@ -449,9 +449,16 @@ def __init__(self, config: AgentConfig, session_id: str | None = None):
                 agent loads/creates a Session in ~/.effgen/sessions/ and
                 appends each run() turn to it.
         """
-        # Fail fast on the bare-constructor trap: ``Agent("groq:model")`` stores
-        # the str unvalidated and only crashes deep in run(). Point users at the
-        # real construction paths instead.
+        # Fail fast on the bare-constructor traps and teach the fix. ``Agent()``
+        # (no args) and ``Agent("groq:model")`` (a bare id) would otherwise be a
+        # cryptic "missing argument" / a str stored unvalidated that crashes deep
+        # in run(). Point users at the real construction paths instead.
+        if config is None:
+            raise TypeError(
+                "Agent() needs an AgentConfig. Build one with "
+                "AgentConfig(name=..., model=...), or use the preset helper "
+                "create_agent('minimal', 'groq:llama-3.1-8b-instant')."
+            )
         if not isinstance(config, AgentConfig):
             raise TypeError(
                 "Agent(config=...) expects an AgentConfig, not "
@@ -501,6 +508,9 @@ def __init__(self, config: AgentConfig, session_id: str | None = None):
                 if config.require_model:
                     # Fail fast on a typo'd id / missing key rather than
                     # returning a working-looking agent (see AgentConfig docs).
+                    # Mark closed first so this never-returned, partially-built
+                    # agent doesn't tail the error with a GC-without-close warning.
+                    self._closed = True
                     logger.error(f"Failed to load model '{self.model_name}': {e}")
                     raise RuntimeError(
                         f"Failed to load model '{self.model_name}': {e}"
@@ -515,6 +525,7 @@ def __init__(self, config: AgentConfig, session_id: str | None = None):
             # BaseModel (e.g. an int, a list, or a Pydantic class). Fail fast
             # with a clear message instead of silently building a model-less
             # agent that only crashes at run().
+            self._closed = True
             raise TypeError(
                 f"AgentConfig.model must be a model-id str or a loaded model "
                 f"instance, not {type(config.model).__name__}. "
@@ -528,6 +539,7 @@ def __init__(self, config: AgentConfig, session_id: str | None = None):
                 # Fail at construction (not on the first run()) so a model-less
                 # agent doesn't look usable. Set require_model=False to defer
                 # model assignment (advanced/sub-agent use).
+                self._closed = True
                 raise ValueError(
                     f"Agent '{config.name}' was created without a model. "
                     "Pass a model id or instance, e.g. "

effgen/core/agent_generation.py

@@ -240,6 +240,10 @@ def _generate(self, prompt: Any, **kwargs) -> dict[str, Any]:
 
         last_error = None
         truncation_detail: dict[str, Any] | None = None
+        # A non-retryable failure (auth/not-found/invalid) is already logged once,
+        # in full, at ERROR below. Track that so the final summary doesn't re-log
+        # the same failure a second time at WARNING (the "logged 3×" noise).
+        nonretryable_logged = False
         total_tokens = 0
         # Whether the caller pinned max_tokens; if they didn't we may grow the
         # budget to recover a reasoning model from a "length"-truncated empty.
@@ -355,6 +359,7 @@ def _generate(self, prompt: Any, **kwargs) -> dict[str, Any]:
                             getattr(current_model, "model_name", "?"),
                             e,
                         )
+                        nonretryable_logged = True
                         break  # stop retrying this model; outer loop may failover
                     if attempt < max_retries - 1:
                         logger.warning(
@@ -388,7 +393,11 @@ def _generate(self, prompt: Any, **kwargs) -> dict[str, Any]:
                 "message": "Model returned an empty response after retries.",
                 "retryable": True,
             }
-        logger.warning("Generation failed: %s", detail["message"])
+        # Avoid re-logging a non-retryable failure already reported at ERROR above.
+        if nonretryable_logged:
+            logger.debug("Generation failed: %s", detail["message"])
+        else:
+            logger.warning("Generation failed: %s", detail["message"])
         return {
             "text": "",
             "tokens_used": total_tokens,

effgen/core/agent_react.py

@@ -46,6 +46,12 @@
 from .agent_runtime import (  # noqa: E402
     _SCAFFOLD_LITERAL_RES,
     _TOOL_ECHO_RE,
+    NUDGE_ALREADY_COMPUTED,
+    NUDGE_CONTINUE,
+    NUDGE_HAVE_ANSWER,
+    NUDGE_HAVE_RESULTS,
+    NUDGE_NO_TOOLS,
+    NUDGE_NOT_USABLE,
     _infer_provider_from_model,
     sanitize_final_answer,
 )
@@ -290,7 +296,7 @@ def _run_single_agent(self,
                     else:
                         batch_observations.append(f"[{_tname}] → Tool not found")
                 # After batch execution, nudge model to synthesize a final answer.
-                scratchpad += "\n[Tool results computed above. Continue or provide Final Answer:]"
+                scratchpad += f"\n{NUDGE_CONTINUE}"
                 _batch_tool_runs += 1
                 parsed = {"thought": "", "action": None, "action_input": None, "final_answer": None}
                 cur_observation = "\n".join(batch_observations)
@@ -371,10 +377,7 @@ def _build_response(
                 logger.info(
                     "Discarding scaffolding-only final answer; continuing loop"
                 )
-                scratchpad += (
-                    "\nObservation: That was not a usable answer. "
-                    "Call the tool correctly or give a plain Final Answer."
-                )
+                scratchpad += f"\nObservation: {NUDGE_NOT_USABLE}"
                 final_answer = None
 
             if final_answer:
@@ -455,8 +458,7 @@ def _build_response(
                     scratchpad += (
                         f"\nAction: {action}"
                         f"\nAction Input: {action_input}"
-                        "\nObservation: You already computed this. "
-                        "Please provide your final response using 'Final Answer:' now."
+                        f"\nObservation: {NUDGE_ALREADY_COMPUTED}"
                     )
                     continue
 
@@ -468,7 +470,7 @@ def _build_response(
                     # Guide it to provide direct answer
                     scratchpad += f"\nAction: {action}"
                     scratchpad += f"\nAction Input: {action_input}"
-                    scratchpad += "\nObservation: No tools available. Please provide your answer directly using 'Final Answer:'."
+                    scratchpad += f"\nObservation: {NUDGE_NO_TOOLS}"
                 else:
                     # Execute tool inside tracing span
                     tool_start = time.time()
@@ -531,15 +533,12 @@ def _build_response(
 
                     # Nudge model to answer when iterations are running low
                     if iterations >= self.config.max_iterations - 2:
-                        scratchpad += "\n[You have the answer from the tool. Please respond with 'Final Answer:' now.]"
+                        scratchpad += f"\n{NUDGE_HAVE_ANSWER}"
                     elif action_call_count >= 1 and not tool_result.startswith("Error executing tool"):
                         # This tool has now run at least twice. The needed data is
                         # almost certainly in the scratchpad — nudge toward a final
                         # answer before the loop drifts into repeated re-planning.
-                        scratchpad += (
-                            "\n[You already have results from this tool. If you have "
-                            "enough information, respond now with 'Final Answer:'.]"
-                        )
+                        scratchpad += f"\n{NUDGE_HAVE_RESULTS}"
 
             else:
                 # No action specified, prompt to continue

effgen/core/agent_runtime.py

@@ -93,15 +93,42 @@ def _safe_float_or_none(value: Any) -> float | None:
         return None
 
 
-# Literal loop-bookkeeping strings the ReAct scaffolding/prompts inject into the
-# scratchpad. These must never reach a user-facing answer. An adjacent newline is
-# consumed when present so removing a mid-line marker doesn't leave an orphan line.
+# Loop-bookkeeping nudges the ReAct scaffolding injects into the scratchpad to
+# steer a stalling model toward a final answer. They are defined here, in ONE
+# place, and imported by the injection sites (see ``agent_react``) so the set of
+# strings a loop can append can never drift out of sync with the set of strings
+# stripped from the user-facing answer below. Adding a new nudge => add a
+# constant here and reference it at the injection site; it is then stripped for
+# free. The injection sites prepend "\n" (and sometimes "Observation: "), which
+# the strip patterns tolerate.
+NUDGE_CONTINUE = "[Tool results computed above. Continue or provide Final Answer:]"
+NUDGE_HAVE_ANSWER = "[You have the answer from the tool. Please respond with 'Final Answer:' now.]"
+NUDGE_HAVE_RESULTS = (
+    "[You already have results from this tool. If you have "
+    "enough information, respond now with 'Final Answer:'.]"
+)
+NUDGE_ALREADY_COMPUTED = (
+    "You already computed this. Please provide your final response "
+    "using 'Final Answer:' now."
+)
+NUDGE_NO_TOOLS = "No tools available. Please provide your answer directly using 'Final Answer:'."
+NUDGE_NOT_USABLE = (
+    "That was not a usable answer. Call the tool correctly or give a "
+    "plain Final Answer."
+)
+
+# Literal loop-bookkeeping strings to strip — every injectable nudge above, plus a
+# couple of defensive bracketed/sub-phrase variants. These must never reach a
+# user-facing answer. An adjacent newline is consumed when present so removing a
+# mid-line marker doesn't leave an orphan line.
 _SCAFFOLD_LITERALS: tuple[str, ...] = (
-    "[Tool results computed above. Continue or provide Final Answer:]",
-    "[You have the answer from the tool. Please respond with 'Final Answer:' now.]",
-    "[You already computed this. Please provide your final response using 'Final Answer:' now.]",
-    "You already computed this. Please provide your final response using 'Final Answer:' now.",
-    "No tools available. Please provide your answer directly using 'Final Answer:'.",
+    NUDGE_CONTINUE,
+    NUDGE_HAVE_ANSWER,
+    NUDGE_HAVE_RESULTS,
+    f"[{NUDGE_ALREADY_COMPUTED}]",
+    NUDGE_ALREADY_COMPUTED,
+    NUDGE_NO_TOOLS,
+    NUDGE_NOT_USABLE,
     "Please provide your final response using 'Final Answer:' now.",
 )
 _SCAFFOLD_LITERAL_RES: tuple[re.Pattern[str], ...] = tuple(