Skip to content

Commit 2a68247

Browse files
committed
fix(integration): run general preset on Gemini, honor workflow edges, classify 413
1 parent 1e767f0 commit 2a68247

13 files changed

Lines changed: 335 additions & 15 deletions

File tree

effgen/cli/_main.py

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2122,16 +2122,22 @@ def _models_info(self, args):
21222122
for k, v in rows.items():
21232123
table.add_row(k, str(v))
21242124
self.console.print(table)
2125-
self.console.print(f"\n[dim]Use: [cyan]effgen run --provider {rec.provider} "
2126-
f"-m {rec.id} \"...\"[/cyan][/dim]")
21272125
else:
21282126
for k, v in rows.items():
21292127
print(f" {k}: {v}")
21302128

2131-
# If the same id is also downloaded locally, surface the local copy and a
2132-
# local invocation alongside the cloud row (don't pretend it's cloud-only).
2129+
cloud_hint = f"effgen run --provider {rec.provider} -m {rec.id} \"...\""
2130+
# If the same id is also downloaded locally, lead with the local engine
2131+
# path (the local block prints its own "Run locally" hint) and show the
2132+
# cloud invocation as an alternative — don't present it as cloud-only.
21332133
if local_entry is not None:
21342134
self._render_local_model_info(self._local_model_payload(local_entry))
2135+
if self.console:
2136+
self.console.print(f"\n[dim]Cloud alternative: [cyan]{cloud_hint}[/cyan][/dim]")
2137+
else:
2138+
print(f"\n Cloud alternative: {cloud_hint}")
2139+
elif self.console:
2140+
self.console.print(f"\n[dim]Use: [cyan]{cloud_hint}[/cyan][/dim]")
21352141
return 0
21362142

21372143
def _models_load(self, args):
@@ -3761,7 +3767,18 @@ def _handle_eval_command(args, cli) -> int:
37613767
cli.print(f" {name:16s}{desc}")
37623768
return 0
37633769

3764-
suite = _resolve_eval_suite(suite_name, difficulty=difficulty, max_cases=max_cases)
3770+
# A bad data file (unknown field, empty, missing) is a user error, not a
3771+
# crash — report it and exit 2 (matching `compare`) instead of a
3772+
# traceback.
3773+
try:
3774+
suite = _resolve_eval_suite(suite_name, difficulty=difficulty, max_cases=max_cases)
3775+
except (ValueError, FileNotFoundError) as exc:
3776+
cli.print(
3777+
f"Could not load suite '{suite_name}' ({exc}). "
3778+
f"Use a built-in suite ({', '.join(list_suites())}) "
3779+
"or a path to a .jsonl/.json file of test cases."
3780+
)
3781+
return 2
37653782

37663783
# Report any narrowing applied
37673784
if difficulty:

effgen/core/workflow.py

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,10 @@ def from_yaml(cls, path: str, agent_factory: Callable[[dict[str, Any]], Any] | N
548548
"""
549549
Load a workflow from a YAML file.
550550
551-
Expected format::
551+
Dependencies can be declared per-node with ``depends_on`` or in a
552+
top-level ``edges`` list; both build the same graph. Each edge is either
553+
a ``[source, target]`` pair or a mapping with ``source``/``target``
554+
(aliases ``from``/``to``) and an optional ``key``::
552555
553556
workflow:
554557
name: my_pipeline
@@ -560,6 +563,10 @@ def from_yaml(cls, path: str, agent_factory: Callable[[dict[str, Any]], Any] | N
560563
agent: summary_agent
561564
depends_on: [search]
562565
566+
# equivalent wiring via a top-level edges block:
567+
# edges:
568+
# - [search, summarize]
569+
563570
Args:
564571
path: Path to the YAML file
565572
agent_factory: Optional callable that receives a node dict and
@@ -568,6 +575,10 @@ def from_yaml(cls, path: str, agent_factory: Callable[[dict[str, Any]], Any] | N
568575
569576
Returns:
570577
A validated WorkflowDAG
578+
579+
An unrecognized top-level key is reported with a warning rather than
580+
dropped silently, so a mis-keyed file (e.g. ``edge:`` instead of
581+
``edges:``) does not validate as a workflow with all its declared wiring.
571582
"""
572583
import yaml # pyyaml is an existing dependency
573584

@@ -577,6 +588,17 @@ def from_yaml(cls, path: str, agent_factory: Callable[[dict[str, Any]], Any] | N
577588
wf_data = data.get("workflow", data)
578589
name = wf_data.get("name", "workflow")
579590

591+
# Surface unknown top-level keys instead of dropping the wiring silently.
592+
_known_top = {"name", "nodes", "edges", "description", "metadata"}
593+
unknown = [k for k in wf_data if k not in _known_top]
594+
if unknown:
595+
logger.warning(
596+
"Workflow '%s': ignoring unrecognized top-level key(s) %s "
597+
"(recognized: %s). Declare dependencies with per-node "
598+
"'depends_on' or a top-level 'edges' list.",
599+
name, sorted(unknown), sorted(_known_top),
600+
)
601+
580602
dag = cls(name=name)
581603

582604
node_defs = wf_data.get("nodes", [])
@@ -589,19 +611,54 @@ def from_yaml(cls, path: str, agent_factory: Callable[[dict[str, Any]], Any] | N
589611
id=nd["id"],
590612
agent=agent,
591613
tools=nd.get("tools", []),
614+
input_keys=nd.get("input_keys", []),
592615
output_key=nd.get("output_key", nd["id"]),
593616
metadata={k: v for k, v in nd.items()
594-
if k not in ("id", "tools", "output_key", "depends_on", "agent")},
617+
if k not in ("id", "tools", "input_keys",
618+
"output_key", "depends_on", "agent")},
595619
)
596620
dag.add_node(node)
597621

598-
# Create edges from depends_on
622+
# Create edges from per-node depends_on ...
599623
for nd in node_defs:
600624
for dep in nd.get("depends_on", []):
601625
dag.connect(dep, nd["id"])
602626

627+
# ... and from a top-level edges list (same graph; both may be present).
628+
for edge in wf_data.get("edges", []):
629+
src, tgt, key = cls._parse_yaml_edge(edge)
630+
dag.connect(src, tgt, key=key)
631+
603632
return dag
604633

634+
@staticmethod
635+
def _parse_yaml_edge(edge: Any) -> tuple[str, str, str | None]:
636+
"""Parse one entry of a YAML ``edges`` list into ``(source, target, key)``.
637+
638+
Accepts a ``[source, target]`` pair or a mapping with ``source``/``target``
639+
(aliases ``from``/``to``) and an optional ``key``.
640+
"""
641+
if isinstance(edge, list | tuple):
642+
if len(edge) < 2:
643+
raise ValueError(
644+
f"Workflow edge {edge!r} must be [source, target]."
645+
)
646+
return str(edge[0]), str(edge[1]), (str(edge[2]) if len(edge) > 2 else None)
647+
if isinstance(edge, dict):
648+
src = edge.get("source", edge.get("from"))
649+
tgt = edge.get("target", edge.get("to"))
650+
if not src or not tgt:
651+
raise ValueError(
652+
f"Workflow edge {edge!r} needs 'source'/'target' "
653+
"(aliases 'from'/'to')."
654+
)
655+
key = edge.get("key")
656+
return str(src), str(tgt), (str(key) if key is not None else None)
657+
raise ValueError(
658+
f"Unsupported workflow edge {edge!r}: use [source, target] or "
659+
"{source, target}."
660+
)
661+
605662
# -- Introspection --
606663

607664
def get_node(self, node_id: str) -> WorkflowNode | None:

effgen/eval/evaluator.py

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ class TestCase:
4141
4242
Attributes:
4343
query: The input query for the agent. ``input=`` is accepted as an
44-
ergonomic alias when constructing a TestCase.
44+
ergonomic alias when constructing a TestCase; :meth:`from_dict`
45+
additionally reads ``prompt`` / ``question``.
4546
expected_output: Expected output text (used for exact/contains/regex).
4647
``expected=`` is accepted as an ergonomic alias.
4748
expected_tools: Tool names the agent should invoke.
@@ -70,17 +71,35 @@ def __post_init__(self) -> None:
7071
self.difficulty = Difficulty(self.difficulty)
7172
if not self.query:
7273
raise ValueError(
73-
"TestCase requires a non-empty 'query' (or 'input=' alias)."
74+
"TestCase requires a non-empty 'query' "
75+
"(aliases: input/prompt/question)."
7476
)
7577

78+
# Field names accepted for the query text, in priority order.
79+
_QUERY_KEYS = ("query", "input", "prompt", "question")
80+
7681
@classmethod
7782
def from_dict(cls, data: dict[str, Any]) -> TestCase:
78-
"""Create a TestCase from a dict (e.g. parsed JSONL line)."""
83+
"""Create a TestCase from a dict (e.g. parsed JSONL line).
84+
85+
The query text is read from ``query`` or any of the aliases
86+
``input`` / ``prompt`` / ``question``. A dict with none of these keys
87+
raises a ``ValueError`` naming the field it needs and the keys it saw.
88+
"""
89+
query = next(
90+
(data[k] for k in cls._QUERY_KEYS if data.get(k)), None
91+
)
92+
if query is None:
93+
raise ValueError(
94+
"each test case needs a 'query' field "
95+
"(aliases: input/prompt/question); got keys: "
96+
f"{sorted(data.keys())}"
97+
)
7998
difficulty = data.get("difficulty", "medium")
8099
if isinstance(difficulty, str):
81100
difficulty = Difficulty(difficulty)
82101
return cls(
83-
query=data["query"],
102+
query=query,
84103
expected_output=data.get("expected", data.get("expected_output", "")),
85104
expected_tools=data.get("tools", data.get("expected_tools", [])),
86105
tags=data.get("tags", []),

effgen/models/errors.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,9 @@ def classify_provider_error(exc: Exception) -> ErrorClass:
521521
return _NOT_FOUND
522522
if status == 429:
523523
return _RATE_LIMITED
524-
if status in (400, 422):
524+
# 413 payload-too-large is a property of the request, not a transient
525+
# rate limit — retrying the same oversized request will not succeed.
526+
if status in (400, 413, 422):
525527
return _INVALID
526528
if status == 408 or status >= 500:
527529
return _TRANSIENT

effgen/models/gemini_adapter.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,15 @@ def _sanitize_schema(cls, schema: Any) -> Any:
314314
cleaned[k] = list(v) if isinstance(v, list | tuple) else v
315315
else:
316316
cleaned[k] = v
317+
# Gemini rejects an ``array`` property that omits ``items``. Default a
318+
# missing element schema to a string so tools that declare a plain
319+
# ``type: array`` parameter are accepted (OpenAI/Groq tolerate the
320+
# omission). Covers every current and future tool without per-tool
321+
# patches.
322+
type_val = cleaned.get("type")
323+
if isinstance(type_val, str) and type_val.lower() == "array" \
324+
and "items" not in cleaned:
325+
cleaned["items"] = {"type": "string"}
317326
return cleaned
318327
if isinstance(schema, list):
319328
return [cls._sanitize_schema(v) for v in schema]

effgen/models/groq_adapter.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,24 @@
4949
_GROQ_MODEL_TYPE_VALUE = "groq"
5050

5151

52+
def _redact_groq_org(message: str) -> str:
53+
"""Remove the caller's organization id from a Groq error body before it is
54+
surfaced (it is an account identifier, not useful for debugging)."""
55+
return re.sub(r"organization `org_[^`]+`", "organization `***`", message)
56+
57+
58+
def _is_request_too_large(message: str, message_lower: str) -> bool:
59+
"""True when a Groq error is a 413 payload-too-large (a single oversized
60+
request), not a 429 rate limit. Groq returns 413 with a ``rate_limit_exceeded``
61+
code for a request over the per-minute token limit, so status + wording are
62+
checked rather than the misleading code."""
63+
return (
64+
"413" in message
65+
or "request too large" in message_lower
66+
or "reduce your message size" in message_lower
67+
)
68+
69+
5270
def _parse_failed_generation_tool_call(message: str) -> dict[str, Any] | None:
5371
"""Extract a tool call from Groq's ``tool_use_failed`` failed_generation text."""
5472
match = re.search(
@@ -441,6 +459,22 @@ def _do_generate(
441459
message=msg,
442460
) from exc
443461

462+
# A 413 payload-too-large is a permanent property of this
463+
# request, not a transient rate limit — fail fast with a
464+
# fix-oriented hint instead of routing it through retry/failover.
465+
if _is_request_too_large(msg, msg_lower):
466+
from effgen.models.errors import InvalidRequestError as _IRE
467+
raise _IRE(
468+
provider="groq",
469+
model_name=self.model_name,
470+
message=(
471+
f"request too large for {self.model_name}: "
472+
f"{_redact_groq_org(msg)} — reduce the request "
473+
"(fewer/smaller tools or shorter input) or use a "
474+
"larger-context model."
475+
),
476+
) from exc
477+
444478
is_rate = "429" in msg or "rate_limit" in msg_lower or "rate limit" in msg_lower
445479
is_server = "500" in msg or "503" in msg or "internal" in msg_lower
446480
is_timeout = "timeout" in msg_lower
@@ -733,6 +767,18 @@ def generate_stream(
733767
model_name=self.model_name,
734768
message=msg,
735769
) from exc
770+
if _is_request_too_large(msg, msg_lower):
771+
from effgen.models.errors import InvalidRequestError as _IRE
772+
raise _IRE(
773+
provider="groq",
774+
model_name=self.model_name,
775+
message=(
776+
f"request too large for {self.model_name}: "
777+
f"{_redact_groq_org(msg)} — reduce the request "
778+
"(fewer/smaller tools or shorter input) or use a "
779+
"larger-context model."
780+
),
781+
) from exc
736782
is_rate = "429" in msg or "rate_limit" in msg_lower or "rate limit" in msg_lower
737783
if is_rate:
738784
from effgen.models._rate_limit import RateLimitExceeded as _RLE

effgen/tools/protocols/mcp_official/client.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,12 @@ def get_effgen_tools(self, prefix: str = "mcp_") -> list[Any]:
352352
This is a **synchronous** accessor (it reads the catalog cached at
353353
:meth:`connect`); call it directly, do not ``await`` it.
354354
355+
Each returned tool is named ``{prefix}{server_tool_name}`` — with the
356+
default ``prefix="mcp_"``, a server tool ``calculator`` is exposed to the
357+
agent as ``mcp_calculator``. Look tools up by that namespaced name; to
358+
reach the original server name pass ``prefix=""`` here, or use
359+
:meth:`get_tool` (which is keyed by the unprefixed name).
360+
355361
.. note::
356362
An ``Agent`` that holds these tools must be driven with
357363
``await agent.run_async(...)``, not the synchronous ``Agent.run()``:
@@ -361,10 +367,12 @@ def get_effgen_tools(self, prefix: str = "mcp_") -> list[Any]:
361367
362368
Args:
363369
prefix: Name prefix for the wrapped tools (avoids collisions with
364-
effGen's builtin tool names).
370+
effGen's builtin tool names). Defaults to ``"mcp_"``; pass
371+
``""`` to keep the original server tool names.
365372
366373
Returns:
367-
List of effGen ``BaseTool`` instances.
374+
List of effGen ``BaseTool`` instances, each named
375+
``{prefix}{server_tool_name}``.
368376
"""
369377
return [
370378
_MCPOfficialToolBridge(tool, self, prefix=prefix)

tests/integration/test_gemini_tools_live.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,3 +170,21 @@ def test_parallel_function_calls_live():
170170
print("Parallel function calls response:", response.output[:300])
171171
finally:
172172
model.unload()
173+
174+
175+
@SKIP
176+
def test_general_preset_array_param_tools_accepted_live():
177+
"""The general preset attaches tools whose parameters include a plain
178+
``type: array`` field (map markers, email attachments). Gemini rejects an
179+
array property without ``items``; the schema sanitizer defaults it so the
180+
request is accepted and the preset runs instead of failing with a 400."""
181+
from effgen.presets import create_agent
182+
183+
agent = create_agent("general", f"gemini:{MODEL}")
184+
response = _run_agent_or_skip_transient(agent, "Say hello in one word.")
185+
_skip_if_gemini_transient(response.output)
186+
# The prior failure surfaced as a 400 "properties[markers].items: missing
187+
# field" before generation — assert the request was accepted and answered.
188+
assert response.success, f"general preset failed on Gemini: {response.metadata.get('error')}"
189+
assert response.output.strip(), "Expected a non-empty answer"
190+
print("general preset on Gemini:", response.output[:120])

tests/models/test_gemini_adapter.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,31 @@ def test_sanitize_schema_strips_unsupported_fields():
4444
}
4545

4646

47+
def test_sanitize_schema_defaults_missing_array_items():
48+
"""Gemini rejects an ``array`` property without ``items``; the sanitizer
49+
defaults the element schema so tools with a plain ``type: array`` param are
50+
accepted (this is what the general preset's maps/email tools declare)."""
51+
raw = {
52+
"type": "object",
53+
"properties": {
54+
"markers": {"type": "array", "description": "map markers"},
55+
"attachments": {"type": "array"},
56+
"typed": {"type": "array", "items": {"type": "integer"}},
57+
},
58+
"required": ["markers"],
59+
}
60+
cleaned = GeminiAdapter._sanitize_schema(raw)
61+
assert cleaned["properties"]["markers"]["items"] == {"type": "string"}
62+
assert cleaned["properties"]["attachments"]["items"] == {"type": "string"}
63+
# An explicit items schema is preserved, not overwritten.
64+
assert cleaned["properties"]["typed"]["items"] == {"type": "integer"}
65+
66+
67+
def test_sanitize_schema_top_level_array_gets_items():
68+
cleaned = GeminiAdapter._sanitize_schema({"type": "array"})
69+
assert cleaned == {"type": "array", "items": {"type": "string"}}
70+
71+
4772
def test_sanitize_schema_preserves_property_names():
4873
"""`properties` keys are user-defined names, not schema fields — keep them."""
4974
raw = {

0 commit comments

Comments
 (0)