feat(appworld): external agent comparison framework with token tracking#100
Open
sami-marreed wants to merge 3 commits into
Open
feat(appworld): external agent comparison framework with token tracking#100sami-marreed wants to merge 3 commits into
sami-marreed wants to merge 3 commits into
Conversation
…acking Introduce Deep Agents, OpenClaw, and Hermes adapters sharing the AppWorld MCP tool harness, wire cache-aware TokenUsageCallback across SDK and external eval paths, and add smoke tests plus a live eval status dashboard for compare runs.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
💤 Files with no reviewable changes (1)
👮 Files not reviewed due to content moderation or server errors (15)
📝 Walkthrough🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (5)
benchmarks/appworld/agents/tools.py (2)
188-206: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winTLS verification bypass is silent when enabled.
ssl_verify=Falsedisables certificate validation for the OpenAI client (static analysis flags this as CWE-295). It's opt-in and defaults to secure, which limits severity, but nothing logs that verification was disabled, making misconfiguration hard to notice in eval output.🔒 Proposed fix: log when TLS verification is disabled
if not ssl_verify: import httpx + logger.warning("TLS certificate verification is disabled for the eval LLM client.") llm_kwargs["http_client"] = httpx.Client(verify=False) # noqa: S501 llm_kwargs["http_async_client"] = httpx.AsyncClient(verify=False) # noqa: S501🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/appworld/agents/tools.py` around lines 188 - 206, Add an explicit warning log in the LLM client setup path in tools.py when ssl_verify is false, so the TLS bypass is visible in eval output. Use the existing environment-driven logic around disable_ssl and ssl_verify in the ChatOpenAI construction branch, and emit a clear message before creating the httpx.Client and httpx.AsyncClient with verify disabled.Source: Linters/SAST tools
104-144: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winTool output payload is injected into the conversation with no size cap.
execute_langchain_toolbuildspayloadfrom the fulljson.dumps(result, ...)and returns it unbounded;tool_loop.pythen appends"Full output:\n{payload}"to the conversation on every step (up to 12 steps). Large paginated AppWorld responses can balloon context size and token cost quickly — counter to this PR's token-tracking goals. The codebase already has a precedent for bounding similar output (res_str[:4000]ineval_appworld_sdk.py's Evolve trajectory save).♻️ Proposed fix: cap payload size
summary = summarize_tool_result(result) try: payload = json.dumps(result, ensure_ascii=False, default=str) except TypeError: payload = str(result) + max_len = 4000 + if len(payload) > max_len: + payload = payload[:max_len] + "... (truncated)" if summary: return f"{summary}\n\nFull output:\n{payload}" return payload🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/appworld/agents/tools.py` around lines 104 - 144, The tool result payload returned by execute_langchain_tool is unbounded, so large tool outputs can flood the conversation context. Update summarize_tool_result/execute_langchain_tool to cap the serialized payload length before returning it, and keep the existing summary prefix while truncating the "Full output" portion to a safe maximum. Use the existing helper patterns in tools.py and align the limit with the project’s other bounded-output precedent.benchmarks/appworld/compare.sh (1)
134-138: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDry-run echo doesn't reflect the actual
eval.shinvocation.The real run builds
EVAL_ARGS=(--agent "$agent" --no-bundle "${FORWARDED_ARGS[@]}")(Line 203), but the dry-run echo at Line 138 omits--no-bundle. Anyone using--dry-runto preview/copy the exact command will get an inaccurate one.🔧 Proposed fix
extra="" if [[ "$agent" == "cuga" ]]; then extra="--sdk " fi - echo " [${config} run ${r}/${RUNS}] ./eval.sh ${extra}--agent ${agent} ${FORWARDED_ARGS[*]}" + echo " [${config} run ${r}/${RUNS}] ./eval.sh ${extra}--agent ${agent} --no-bundle ${FORWARDED_ARGS[*]}"Also applies to: 203-209
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/appworld/compare.sh` around lines 134 - 138, The dry-run message in compare.sh is out of sync with the real eval.sh invocation built in the loop around EVAL_ARGS, since it omits the required --no-bundle flag. Update the echo that prints the preview command to mirror the actual argument construction used by the run path, including --no-bundle alongside --agent, and keep the cuga-specific --sdk handling and forwarded args consistent so --dry-run shows the exact command users can copy.benchmarks/appworld/tests/test_agent_adapters.py (2)
120-172: 📐 Maintainability & Code Quality | 🔵 TrivialTest coverage gap: OpenClaw adapter untested, DeepAgents ReAct fallback untested.
The stack description states DeepAgents adapter has "tool-binding decision logic ... with ReAct fallback" and that an OpenClaw adapter exists, but this file only exercises the DeepAgents happy path (native tool-calling message) and the Hermes wrapper. No test targets
OpenClawAppWorldAgent, and no test forces the DeepAgents fallback branch (e.g., when the LLM can't bind tools and the loop falls back torun_tool_react_loop).Do you want me to draft tests for the OpenClaw adapter and the DeepAgents fallback path?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/appworld/tests/test_agent_adapters.py` around lines 120 - 172, Add tests to cover the missing adapter paths in the existing adapter test module: exercise OpenClawAppWorldAgent through create_appworld_agent or its direct invoke path, and add a DeepAgentsAppWorldAgent case that forces the ReAct fallback instead of the native tool-calling branch. Use the existing test patterns around DeepAgentsAppWorldAgent, create_appworld_agent, and run_tool_react_loop to verify the fallback is selected and returns an AppWorldInvokeResult, and ensure the OpenClaw adapter is instantiated and invoked with mocked tools like the current _MockTool-based tests.
24-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
_MockToolhelper across test files.Identical
_MockToolclass is redefined inbenchmarks/appworld/tests/test_token_harness.py(lines 15-23). Consider extracting a shared fixture/helper into aconftest.pyunderbenchmarks/appworld/tests/.♻️ Suggested extraction
# benchmarks/appworld/tests/conftest.py from typing import Any class _MockTool: def __init__(self, name: str, result: Any = "ok"): self.name = name self.description = f"Mock {name}" self._result = result async def ainvoke(self, args: dict[str, Any]) -> Any: return {"tool": self.name, "args": args, "result": self._result}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/appworld/tests/test_agent_adapters.py` around lines 24 - 31, The _MockTool helper is duplicated in multiple benchmark test files, so move the shared implementation into a common test helper in benchmarks/appworld/tests/conftest.py and update both test modules to import it instead of redefining it. Keep the class behavior identical, including __init__ and ainvoke, so references in test_agent_adapters and test_token_harness continue to work with the shared symbol.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmarks/appworld/agents/deepagents.py`:
- Around line 182-191: `_ensure_agent()` is being called before the `try`, so
failures from `create_deep_agent(...)` and tool binding bypass the ReAct
fallback in `run`. Move the `self._ensure_agent()` call into the existing
fallback-protected block in `DeepAgents.run`, and keep the rest of the invoke
setup unchanged so any agent निर्माण errors are caught and routed through the
fallback handler.
In `@benchmarks/appworld/agents/factory.py`:
- Around line 5-25: The module-level imports in create_appworld_agent force
optional agent packages to be installed even when only one agent is used. Move
the DeepAgentsAppWorldAgent, OpenClawAppWorldAgent, and HermesAppWorldAgent
imports inside the corresponding create_appworld_agent branches so only the
requested adapter is loaded at runtime. Keep EXTERNAL_AGENT_NAMES and the
normalized name dispatch logic intact, and use the existing
create_appworld_agent function as the main location for the deferred imports.
In `@benchmarks/appworld/agents/openclaw.py`:
- Around line 163-173: The OpenClaw handling in the invoke path returns too
early when `tool_request` is present without a final answer, so the tool
observation never gets fed back into the model. Update the flow in the
`AppWorldInvokeResult`/`execute_tool_by_name` branch to continue the loop after
executing the native tool, then pass the observation back into OpenClaw until a
`Final Answer:` is produced. Keep the current `tool_calls` tracking behavior,
but only return once `final_answer` is available instead of using the original
`openclaw_result` as the answer.
In `@benchmarks/appworld/agents/tools.py`:
- Around line 95-101: normalize_tool_args currently copies args and appends
snake_case aliases, leaving both the original camelCase key and the converted
key in the returned dict, which can break execute_langchain_tool when it calls
tool(**normalized_args) or invoke/ainvoke validation. Update normalize_tool_args
to replace uppercase-containing keys with their snake_case form instead of
preserving the original, so the normalized result contains only one version of
each argument name. Keep the behavior localized to normalize_tool_args and
ensure execute_langchain_tool receives a deduplicated argument map.
In `@benchmarks/appworld/appworld_eval_external.py`:
- Around line 274-292: The Langfuse flush is only happening after the
try/except/finally block, so it is skipped on interrupted or failed runs in the
main evaluation flow. Move the flush into the cleanup path handled by the
evaluation runner (around the try/finally in the task loop) so it always
executes before re-raising, including the KeyboardInterrupt and Exception paths;
use the existing flush_langfuse and self.langfuse_handler symbols to place it in
the shared teardown logic.
In `@benchmarks/appworld/README.md`:
- Line 244: The `--model-profile` documentation is stale and missing the newer
profiles supported by `scripts/model_profiles.sh`. Update the CLI options table
in the README entry for `--model-profile` so it includes `gpt5` and `gpt5.2`
alongside the existing profiles, and make sure the example/value list stays in
sync with the profile names used by `model_profiles.sh`.
In `@benchmarks/appworld/smoke_external_agents.py`:
- Around line 160-164: Reject empty agent selections in the smoke runner: in
smoke_external_agents.py where args.agents is parsed into agent_names and
validated against EXTERNAL_AGENT_NAMES, add a check that fails fast when the
parsed list is empty (for example after splitting/stripping --agents ""). Return
a non-zero exit code and log a clear error before running any checks, alongside
the existing unknown-agent validation in the agent_names/unknown handling block.
- Around line 111-117: The smoke test in the `ok` check is too strict for
`--native-sdk` because `token_callback.total_tokens` and
`token_callback.llm_calls` may stay at zero even when the model and tool call
succeed. Update the logic around the `smoke_external_agents.py` result check to
make the token assertions conditional on non-native SDK mode, while still
requiring `result.error is None`, a success answer, and at least one tool call;
use the existing `token_callback` and `ok` symbols to locate the check.
In `@benchmarks/appworld/smoke_external.sh`:
- Around line 22-27: The smoke script currently sources .env as shell code,
which is redundant and can execute non-shell dotenv content; update
smoke_external.sh to stop using source around the PROJECT_ROOT/.env block and
let the Python smoke path load environment variables via python-dotenv instead.
Keep the existing PROJECT_ROOT and .env detection flow, but remove the shell
execution behavior so the script only reads configuration indirectly through the
Python entrypoint.
In `@benchmarks/helpers/token_usage.py`:
- Around line 38-56: The usage aggregation in _add_usage_from_mapping currently
returns True even when only cache tokens are present, which prevents
_merge_usage_maps/token_usage from being applied and drops prompt/completion
counts. Update _add_usage_from_mapping so it only marks the mapping as handled
after merging all complementary usage fields, and make sure the logic in
token_usage and the related helper path around _merge_usage_maps combines
cache-only and token-only maps before deciding the call is counted.
In `@pyproject.toml`:
- Line 44: Revert the committed machine-local appworld entries from
pyproject.toml: the [tool.uv.sources] appworld editable path and the
[dependency-groups].appworld entry should not be tracked because they are only
generated locally by setup_appworld.sh. Keep these changes out of the committed
configuration so fresh checkouts and CI do not point at
benchmarks/appworld/appworld, and ensure the tracked pyproject.toml matches the
documented policy enforced by scripts/check_no_local_appworld.sh.
---
Nitpick comments:
In `@benchmarks/appworld/agents/tools.py`:
- Around line 188-206: Add an explicit warning log in the LLM client setup path
in tools.py when ssl_verify is false, so the TLS bypass is visible in eval
output. Use the existing environment-driven logic around disable_ssl and
ssl_verify in the ChatOpenAI construction branch, and emit a clear message
before creating the httpx.Client and httpx.AsyncClient with verify disabled.
- Around line 104-144: The tool result payload returned by
execute_langchain_tool is unbounded, so large tool outputs can flood the
conversation context. Update summarize_tool_result/execute_langchain_tool to cap
the serialized payload length before returning it, and keep the existing summary
prefix while truncating the "Full output" portion to a safe maximum. Use the
existing helper patterns in tools.py and align the limit with the project’s
other bounded-output precedent.
In `@benchmarks/appworld/compare.sh`:
- Around line 134-138: The dry-run message in compare.sh is out of sync with the
real eval.sh invocation built in the loop around EVAL_ARGS, since it omits the
required --no-bundle flag. Update the echo that prints the preview command to
mirror the actual argument construction used by the run path, including
--no-bundle alongside --agent, and keep the cuga-specific --sdk handling and
forwarded args consistent so --dry-run shows the exact command users can copy.
In `@benchmarks/appworld/tests/test_agent_adapters.py`:
- Around line 120-172: Add tests to cover the missing adapter paths in the
existing adapter test module: exercise OpenClawAppWorldAgent through
create_appworld_agent or its direct invoke path, and add a
DeepAgentsAppWorldAgent case that forces the ReAct fallback instead of the
native tool-calling branch. Use the existing test patterns around
DeepAgentsAppWorldAgent, create_appworld_agent, and run_tool_react_loop to
verify the fallback is selected and returns an AppWorldInvokeResult, and ensure
the OpenClaw adapter is instantiated and invoked with mocked tools like the
current _MockTool-based tests.
- Around line 24-31: The _MockTool helper is duplicated in multiple benchmark
test files, so move the shared implementation into a common test helper in
benchmarks/appworld/tests/conftest.py and update both test modules to import it
instead of redefining it. Keep the class behavior identical, including __init__
and ainvoke, so references in test_agent_adapters and test_token_harness
continue to work with the shared symbol.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3c2f7219-0e20-49e7-a930-93b967af15e3
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (32)
.env.example.gitignorebenchmarks/appworld/README.mdbenchmarks/appworld/agents/__init__.pybenchmarks/appworld/agents/base.pybenchmarks/appworld/agents/deepagents.pybenchmarks/appworld/agents/factory.pybenchmarks/appworld/agents/final_answer.pybenchmarks/appworld/agents/hermes.pybenchmarks/appworld/agents/openclaw.pybenchmarks/appworld/agents/tool_loop.pybenchmarks/appworld/agents/tools.pybenchmarks/appworld/appworld_eval_external.pybenchmarks/appworld/compare.shbenchmarks/appworld/eval.shbenchmarks/appworld/eval_appworld_sdk.pybenchmarks/appworld/experiments/outputs/.gitkeepbenchmarks/appworld/experiments/status.htmlbenchmarks/appworld/smoke_external.shbenchmarks/appworld/smoke_external_agents.pybenchmarks/appworld/tests/test_agent_adapters.pybenchmarks/appworld/tests/test_token_harness.pybenchmarks/appworld/utils/appworld_harness.pybenchmarks/helpers/eval_status.pybenchmarks/helpers/sdk_eval_helpers.pybenchmarks/helpers/tests/test_eval_status.pybenchmarks/helpers/tests/test_token_usage.pybenchmarks/helpers/token_usage.pypyproject.tomlscripts/eval.shscripts/eval_status.shscripts/model_profiles.sh
Comment on lines
+182
to
+191
| agent = self._ensure_agent() | ||
| user_content = intent.strip() | ||
| if user_context.strip(): | ||
| user_content = f"{user_content}\n\nContext:\n{user_context.strip()}" | ||
|
|
||
| invoke_config: dict[str, Any] = {"configurable": {"thread_id": thread_id}} | ||
| if config: | ||
| invoke_config.update(config) | ||
|
|
||
| try: |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Wrap DeepAgents creation in the fallback handler.
_ensure_agent() can fail while create_deep_agent(...) binds tools, but it runs before the try, so those failures bypass the ReAct fallback below.
Proposed fix
- agent = self._ensure_agent()
user_content = intent.strip()
if user_context.strip():
user_content = f"{user_content}\n\nContext:\n{user_context.strip()}"
invoke_config: dict[str, Any] = {"configurable": {"thread_id": thread_id}}
if config:
invoke_config.update(config)
try:
+ agent = self._ensure_agent()
if hasattr(agent, "ainvoke"):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| agent = self._ensure_agent() | |
| user_content = intent.strip() | |
| if user_context.strip(): | |
| user_content = f"{user_content}\n\nContext:\n{user_context.strip()}" | |
| invoke_config: dict[str, Any] = {"configurable": {"thread_id": thread_id}} | |
| if config: | |
| invoke_config.update(config) | |
| try: | |
| user_content = intent.strip() | |
| if user_context.strip(): | |
| user_content = f"{user_content}\n\nContext:\n{user_context.strip()}" | |
| invoke_config: dict[str, Any] = {"configurable": {"thread_id": thread_id}} | |
| if config: | |
| invoke_config.update(config) | |
| try: | |
| agent = self._ensure_agent() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/appworld/agents/deepagents.py` around lines 182 - 191,
`_ensure_agent()` is being called before the `try`, so failures from
`create_deep_agent(...)` and tool binding bypass the ReAct fallback in `run`.
Move the `self._ensure_agent()` call into the existing fallback-protected block
in `DeepAgents.run`, and keep the rest of the invoke setup unchanged so any
agent निर्माण errors are caught and routed through the fallback handler.
Comment on lines
+5
to
+25
| from typing import Any | ||
|
|
||
| from benchmarks.appworld.agents.base import AppWorldAgent | ||
| from benchmarks.appworld.agents.deepagents import DeepAgentsAppWorldAgent | ||
| from benchmarks.appworld.agents.hermes import HermesAppWorldAgent | ||
| from benchmarks.appworld.agents.openclaw import OpenClawAppWorldAgent | ||
|
|
||
| EXTERNAL_AGENT_NAMES = frozenset({"deepagents", "openclaw", "hermes"}) | ||
|
|
||
|
|
||
| def create_appworld_agent(name: str, tools: list[Any], **kwargs: Any) -> AppWorldAgent: | ||
| normalized = name.strip().lower() | ||
| if normalized == "deepagents": | ||
| return DeepAgentsAppWorldAgent(tools=tools, **kwargs) | ||
| if normalized == "openclaw": | ||
| return OpenClawAppWorldAgent(tools=tools, **kwargs) | ||
| if normalized == "hermes": | ||
| return HermesAppWorldAgent(tools=tools, **kwargs) | ||
| raise ValueError( | ||
| f"Unknown external agent {name!r}. Supported: {', '.join(sorted(EXTERNAL_AGENT_NAMES))}" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Eager top-level imports force all three external packages to be installed for any agent.
create_appworld_agent imports DeepAgentsAppWorldAgent, HermesAppWorldAgent, and OpenClawAppWorldAgent unconditionally at module load. If deepagents or openclaw are optional dependency groups (per the pyproject.toml dependency-group updates in this stack), simply importing benchmarks.appworld.agents — e.g., to run a Hermes-only eval or smoke test — will raise ImportError unless every external package is installed, defeating the purpose of splitting them into separate groups.
🔧 Proposed fix: defer imports per branch
-from benchmarks.appworld.agents.base import AppWorldAgent
-from benchmarks.appworld.agents.deepagents import DeepAgentsAppWorldAgent
-from benchmarks.appworld.agents.hermes import HermesAppWorldAgent
-from benchmarks.appworld.agents.openclaw import OpenClawAppWorldAgent
+from benchmarks.appworld.agents.base import AppWorldAgent
EXTERNAL_AGENT_NAMES = frozenset({"deepagents", "openclaw", "hermes"})
def create_appworld_agent(name: str, tools: list[Any], **kwargs: Any) -> AppWorldAgent:
normalized = name.strip().lower()
if normalized == "deepagents":
+ from benchmarks.appworld.agents.deepagents import DeepAgentsAppWorldAgent
return DeepAgentsAppWorldAgent(tools=tools, **kwargs)
if normalized == "openclaw":
+ from benchmarks.appworld.agents.openclaw import OpenClawAppWorldAgent
return OpenClawAppWorldAgent(tools=tools, **kwargs)
if normalized == "hermes":
+ from benchmarks.appworld.agents.hermes import HermesAppWorldAgent
return HermesAppWorldAgent(tools=tools, **kwargs)
raise ValueError(
f"Unknown external agent {name!r}. Supported: {', '.join(sorted(EXTERNAL_AGENT_NAMES))}"
)#!/bin/bash
# Confirm whether deepagents/openclaw packages are imported at module top-level in their adapters,
# and whether they're declared as optional/extra dependency groups in pyproject.toml.
rg -n '^import|^from' benchmarks/appworld/agents/deepagents.py benchmarks/appworld/agents/openclaw.py
rg -n 'deepagents|openclaw' pyproject.toml🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/appworld/agents/factory.py` around lines 5 - 25, The module-level
imports in create_appworld_agent force optional agent packages to be installed
even when only one agent is used. Move the DeepAgentsAppWorldAgent,
OpenClawAppWorldAgent, and HermesAppWorldAgent imports inside the corresponding
create_appworld_agent branches so only the requested adapter is loaded at
runtime. Keep EXTERNAL_AGENT_NAMES and the normalized name dispatch logic
intact, and use the existing create_appworld_agent function as the main location
for the deferred imports.
Comment on lines
+163
to
+173
| if final_answer is not None or tool_request is not None: | ||
| tool_calls: list[dict[str, Any]] = [] | ||
| if tool_request: | ||
| name, args = tool_request | ||
| observation = await execute_tool_by_name(self.tools, name, args) | ||
| tool_calls.append({"name": name, "arguments": args, "result": observation}) | ||
| return AppWorldInvokeResult( | ||
| answer=final_answer or openclaw_result, | ||
| tool_calls=tool_calls if track_tool_calls else [], | ||
| react_steps=1, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don’t return after a single native tool request.
When OpenClaw emits a tool request without a final answer, this executes the tool once but returns the original tool-request text as answer; the observation is never fed back to produce Final Answer:.
Proposed fix
- if final_answer is not None or tool_request is not None:
- tool_calls: list[dict[str, Any]] = []
- if tool_request:
- name, args = tool_request
- observation = await execute_tool_by_name(self.tools, name, args)
- tool_calls.append({"name": name, "arguments": args, "result": observation})
+ if final_answer is not None:
return AppWorldInvokeResult(
- answer=final_answer or openclaw_result,
- tool_calls=tool_calls if track_tool_calls else [],
+ answer=final_answer,
+ tool_calls=[],
react_steps=1,
)
+ if tool_request is not None:
+ logger.info("OpenClaw requested a local tool; falling back to shared ReAct loop")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if final_answer is not None or tool_request is not None: | |
| tool_calls: list[dict[str, Any]] = [] | |
| if tool_request: | |
| name, args = tool_request | |
| observation = await execute_tool_by_name(self.tools, name, args) | |
| tool_calls.append({"name": name, "arguments": args, "result": observation}) | |
| return AppWorldInvokeResult( | |
| answer=final_answer or openclaw_result, | |
| tool_calls=tool_calls if track_tool_calls else [], | |
| react_steps=1, | |
| ) | |
| if final_answer is not None: | |
| return AppWorldInvokeResult( | |
| answer=final_answer, | |
| tool_calls=[], | |
| react_steps=1, | |
| ) | |
| if tool_request is not None: | |
| logger.info("OpenClaw requested a local tool; falling back to shared ReAct loop") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/appworld/agents/openclaw.py` around lines 163 - 173, The OpenClaw
handling in the invoke path returns too early when `tool_request` is present
without a final answer, so the tool observation never gets fed back into the
model. Update the flow in the `AppWorldInvokeResult`/`execute_tool_by_name`
branch to continue the loop after executing the native tool, then pass the
observation back into OpenClaw until a `Final Answer:` is produced. Keep the
current `tool_calls` tracking behavior, but only return once `final_answer` is
available instead of using the original `openclaw_result` as the answer.
Comment on lines
+95
to
+101
| def normalize_tool_args(args: dict[str, Any]) -> dict[str, Any]: | ||
| normalized = dict(args) | ||
| for key in list(args.keys()): | ||
| snake_key = re.sub(r"(?<!^)(?=[A-Z])", "_", key).lower() | ||
| if snake_key not in normalized: | ||
| normalized[snake_key] = args[key] | ||
| return normalized |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
normalize_tool_args keeps both the original and snake_case keys, risking duplicate/unexpected-keyword failures.
For any key containing uppercase characters, the function adds a snake_case alias alongside the original key rather than replacing it, so the returned dict contains both someArg and some_arg with the same value. When execute_langchain_tool (Line 125-126) falls back to tool(**normalized_args) for plain-callable tools, this can raise TypeError for an unexpected/duplicate keyword. It can also trip strict pydantic args_schema validation for ainvoke/invoke paths.
🔧 Proposed fix: convert keys instead of duplicating them
def normalize_tool_args(args: dict[str, Any]) -> dict[str, Any]:
- normalized = dict(args)
- for key in list(args.keys()):
- snake_key = re.sub(r"(?<!^)(?=[A-Z])", "_", key).lower()
- if snake_key not in normalized:
- normalized[snake_key] = args[key]
- return normalized
+ normalized: dict[str, Any] = {}
+ for key, value in args.items():
+ snake_key = re.sub(r"(?<!^)(?=[A-Z])", "_", key).lower()
+ normalized[snake_key] = value
+ return normalized📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def normalize_tool_args(args: dict[str, Any]) -> dict[str, Any]: | |
| normalized = dict(args) | |
| for key in list(args.keys()): | |
| snake_key = re.sub(r"(?<!^)(?=[A-Z])", "_", key).lower() | |
| if snake_key not in normalized: | |
| normalized[snake_key] = args[key] | |
| return normalized | |
| def normalize_tool_args(args: dict[str, Any]) -> dict[str, Any]: | |
| normalized: dict[str, Any] = {} | |
| for key, value in args.items(): | |
| snake_key = re.sub(r"(?<!^)(?=[A-Z])", "_", key).lower() | |
| normalized[snake_key] = value | |
| return normalized |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/appworld/agents/tools.py` around lines 95 - 101,
normalize_tool_args currently copies args and appends snake_case aliases,
leaving both the original camelCase key and the converted key in the returned
dict, which can break execute_langchain_tool when it calls
tool(**normalized_args) or invoke/ainvoke validation. Update normalize_tool_args
to replace uppercase-containing keys with their snake_case form instead of
preserving the original, so the normalized result contains only one version of
each argument name. Keep the behavior localized to normalize_tool_args and
ensure execute_langchain_tool receives a deduplicated argument map.
Comment on lines
+274
to
+292
| try: | ||
| for i, tid in enumerate(task_ids, 1): | ||
| logger.info(f"\n[{i}/{len(task_ids)}] Task {tid}") | ||
| self.status_writer.start_task(tid, i) | ||
| result = await self.evaluate_task(tid, task_index=i) | ||
| self.results.append(result) | ||
| self.status_writer.finish_task(result) | ||
| if i < len(task_ids): | ||
| await asyncio.sleep(0.5) | ||
| except KeyboardInterrupt: | ||
| self._run_status = "interrupted" | ||
| raise | ||
| except Exception: | ||
| self._run_status = "failed" | ||
| raise | ||
| finally: | ||
| self.status_writer.finish_run(self._run_status) | ||
|
|
||
| flush_langfuse(self.langfuse_handler) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Flush Langfuse in the failure path too.
flush_langfuse() is skipped when the loop raises and the exception is re-raised, so failed/interrupted runs can lose pending trace data.
Proposed fix
finally:
self.status_writer.finish_run(self._run_status)
-
- flush_langfuse(self.langfuse_handler)
+ try:
+ flush_langfuse(self.langfuse_handler)
+ except Exception as e:
+ logger.warning(f"Failed to flush Langfuse: {e}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try: | |
| for i, tid in enumerate(task_ids, 1): | |
| logger.info(f"\n[{i}/{len(task_ids)}] Task {tid}") | |
| self.status_writer.start_task(tid, i) | |
| result = await self.evaluate_task(tid, task_index=i) | |
| self.results.append(result) | |
| self.status_writer.finish_task(result) | |
| if i < len(task_ids): | |
| await asyncio.sleep(0.5) | |
| except KeyboardInterrupt: | |
| self._run_status = "interrupted" | |
| raise | |
| except Exception: | |
| self._run_status = "failed" | |
| raise | |
| finally: | |
| self.status_writer.finish_run(self._run_status) | |
| flush_langfuse(self.langfuse_handler) | |
| try: | |
| for i, tid in enumerate(task_ids, 1): | |
| logger.info(f"\n[{i}/{len(task_ids)}] Task {tid}") | |
| self.status_writer.start_task(tid, i) | |
| result = await self.evaluate_task(tid, task_index=i) | |
| self.results.append(result) | |
| self.status_writer.finish_task(result) | |
| if i < len(task_ids): | |
| await asyncio.sleep(0.5) | |
| except KeyboardInterrupt: | |
| self._run_status = "interrupted" | |
| raise | |
| except Exception: | |
| self._run_status = "failed" | |
| raise | |
| finally: | |
| self.status_writer.finish_run(self._run_status) | |
| try: | |
| flush_langfuse(self.langfuse_handler) | |
| except Exception as e: | |
| logger.warning(f"Failed to flush Langfuse: {e}") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/appworld/appworld_eval_external.py` around lines 274 - 292, The
Langfuse flush is only happening after the try/except/finally block, so it is
skipped on interrupted or failed runs in the main evaluation flow. Move the
flush into the cleanup path handled by the evaluation runner (around the
try/finally in the task loop) so it always executes before re-raising, including
the KeyboardInterrupt and Exception paths; use the existing flush_langfuse and
self.langfuse_handler symbols to place it in the shared teardown logic.
Comment on lines
+111
to
+117
| ok = ( | ||
| result.error is None | ||
| and "success" in (result.answer or "").lower() | ||
| and len(result.tool_calls or []) >= 1 | ||
| and token_callback.total_tokens > 0 | ||
| and token_callback.llm_calls >= 1 | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Relax token assertions for native SDK mode.
With --native-sdk, native OpenClaw/Hermes calls do not necessarily trigger LangChain callbacks, so a successful model/tool smoke can still fail on total_tokens/llm_calls.
Proposed fix
+ require_langchain_usage = prefer_eval_llm or agent_name == "deepagents"
ok = (
result.error is None
and "success" in (result.answer or "").lower()
and len(result.tool_calls or []) >= 1
- and token_callback.total_tokens > 0
- and token_callback.llm_calls >= 1
+ and (
+ not require_langchain_usage
+ or (token_callback.total_tokens > 0 and token_callback.llm_calls >= 1)
+ )
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ok = ( | |
| result.error is None | |
| and "success" in (result.answer or "").lower() | |
| and len(result.tool_calls or []) >= 1 | |
| and token_callback.total_tokens > 0 | |
| and token_callback.llm_calls >= 1 | |
| ) | |
| require_langchain_usage = prefer_eval_llm or agent_name == "deepagents" | |
| ok = ( | |
| result.error is None | |
| and "success" in (result.answer or "").lower() | |
| and len(result.tool_calls or []) >= 1 | |
| and ( | |
| not require_langchain_usage | |
| or (token_callback.total_tokens > 0 and token_callback.llm_calls >= 1) | |
| ) | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/appworld/smoke_external_agents.py` around lines 111 - 117, The
smoke test in the `ok` check is too strict for `--native-sdk` because
`token_callback.total_tokens` and `token_callback.llm_calls` may stay at zero
even when the model and tool call succeed. Update the logic around the
`smoke_external_agents.py` result check to make the token assertions conditional
on non-native SDK mode, while still requiring `result.error is None`, a success
answer, and at least one tool call; use the existing `token_callback` and `ok`
symbols to locate the check.
Comment on lines
+160
to
+164
| agent_names = [a.strip().lower() for a in args.agents.split(",") if a.strip()] | ||
| unknown = set(agent_names) - EXTERNAL_AGENT_NAMES | ||
| if unknown: | ||
| logger.error(f"Unknown agents: {unknown}") | ||
| return 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject empty agent selections.
--agents "" currently runs no checks and exits successfully with 0/0 passed, which can mask a misconfigured smoke run.
Proposed fix
agent_names = [a.strip().lower() for a in args.agents.split(",") if a.strip()]
+ if not agent_names:
+ logger.error("No agents selected")
+ return 1
unknown = set(agent_names) - EXTERNAL_AGENT_NAMES📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| agent_names = [a.strip().lower() for a in args.agents.split(",") if a.strip()] | |
| unknown = set(agent_names) - EXTERNAL_AGENT_NAMES | |
| if unknown: | |
| logger.error(f"Unknown agents: {unknown}") | |
| return 1 | |
| agent_names = [a.strip().lower() for a in args.agents.split(",") if a.strip()] | |
| if not agent_names: | |
| logger.error("No agents selected") | |
| return 1 | |
| unknown = set(agent_names) - EXTERNAL_AGENT_NAMES | |
| if unknown: | |
| logger.error(f"Unknown agents: {unknown}") | |
| return 1 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/appworld/smoke_external_agents.py` around lines 160 - 164, Reject
empty agent selections in the smoke runner: in smoke_external_agents.py where
args.agents is parsed into agent_names and validated against
EXTERNAL_AGENT_NAMES, add a check that fails fast when the parsed list is empty
(for example after splitting/stripping --agents ""). Return a non-zero exit code
and log a clear error before running any checks, alongside the existing
unknown-agent validation in the agent_names/unknown handling block.
Comment on lines
+22
to
+27
| if [ -f "$PROJECT_ROOT/.env" ]; then | ||
| set -a | ||
| # shellcheck disable=SC1091 | ||
| source "$PROJECT_ROOT/.env" | ||
| set +a | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid sourcing .env as shell code.
The Python smoke module already uses python-dotenv; sourcing .env here is redundant and can execute shell syntax from a dotenv file.
Proposed fix
-if [ -f "$PROJECT_ROOT/.env" ]; then
- set -a
- # shellcheck disable=SC1091
- source "$PROJECT_ROOT/.env"
- set +a
-fi
-
echo "Smoke test — external agents (no CUGA / no AppWorld)"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if [ -f "$PROJECT_ROOT/.env" ]; then | |
| set -a | |
| # shellcheck disable=SC1091 | |
| source "$PROJECT_ROOT/.env" | |
| set +a | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/appworld/smoke_external.sh` around lines 22 - 27, The smoke script
currently sources .env as shell code, which is redundant and can execute
non-shell dotenv content; update smoke_external.sh to stop using source around
the PROJECT_ROOT/.env block and let the Python smoke path load environment
variables via python-dotenv instead. Keep the existing PROJECT_ROOT and .env
detection flow, but remove the shell execution behavior so the script only reads
configuration indirectly through the Python entrypoint.
Comment on lines
+38
to
+56
| def _add_usage_from_mapping(mapping: dict, callback: "TokenUsageCallback") -> bool: | ||
| if not mapping: | ||
| return False | ||
| input_tokens = _int_or_zero( | ||
| mapping.get("input_tokens") | ||
| or mapping.get("prompt_tokens") | ||
| or mapping.get("prompt_token_count") | ||
| ) | ||
| output_tokens = _int_or_zero( | ||
| mapping.get("output_tokens") | ||
| or mapping.get("completion_tokens") | ||
| or mapping.get("completion_token_count") | ||
| ) | ||
| cache_tokens = _cache_tokens_from_mapping(mapping) | ||
| if input_tokens or output_tokens or cache_tokens: | ||
| callback.input_tokens += input_tokens | ||
| callback.output_tokens += output_tokens | ||
| callback.cache_input_tokens += cache_tokens | ||
| return True |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Merge complementary usage maps before marking the call counted.
If llm_output["usage"] contains only cache fields, _add_usage_from_mapping() returns True, so token_usage is skipped and prompt/completion tokens are dropped.
Proposed direction
+def _usage_counts_from_mapping(mapping: dict) -> tuple[int, int, int]:
+ if not mapping:
+ return 0, 0, 0
+ input_tokens = _int_or_zero(
+ mapping.get("input_tokens")
+ or mapping.get("prompt_tokens")
+ or mapping.get("prompt_token_count")
+ )
+ output_tokens = _int_or_zero(
+ mapping.get("output_tokens")
+ or mapping.get("completion_tokens")
+ or mapping.get("completion_token_count")
+ )
+ return input_tokens, output_tokens, _cache_tokens_from_mapping(mapping)
+
+
def _add_usage_from_mapping(mapping: dict, callback: "TokenUsageCallback") -> bool:
- if not mapping:
- return False
- input_tokens = _int_or_zero(
- mapping.get("input_tokens")
- or mapping.get("prompt_tokens")
- or mapping.get("prompt_token_count")
- )
- output_tokens = _int_or_zero(
- mapping.get("output_tokens")
- or mapping.get("completion_tokens")
- or mapping.get("completion_token_count")
- )
- cache_tokens = _cache_tokens_from_mapping(mapping)
+ input_tokens, output_tokens, cache_tokens = _usage_counts_from_mapping(mapping)
if input_tokens or output_tokens or cache_tokens:
callback.input_tokens += input_tokens
callback.output_tokens += output_tokens
callback.cache_input_tokens += cache_tokens if response.llm_output:
out = response.llm_output
- counted = _add_usage_from_mapping(out.get("usage", {}) or {}, outer)
- if not counted:
- counted = _add_usage_from_mapping(out.get("token_usage", {}) or {}, outer)
- if not counted:
- cache_only = _cache_tokens_from_mapping(out.get("usage", {}) or {})
- cache_only += _cache_tokens_from_mapping(out.get("token_usage", {}) or {})
- if cache_only:
- outer.cache_input_tokens += cache_only
- counted = True
+ usage_counts = _usage_counts_from_mapping(out.get("usage", {}) or {})
+ token_usage_counts = _usage_counts_from_mapping(out.get("token_usage", {}) or {})
+ input_tokens = usage_counts[0] or token_usage_counts[0]
+ output_tokens = usage_counts[1] or token_usage_counts[1]
+ cache_tokens = usage_counts[2] or token_usage_counts[2]
+ if input_tokens or output_tokens or cache_tokens:
+ outer.input_tokens += input_tokens
+ outer.output_tokens += output_tokens
+ outer.cache_input_tokens += cache_tokens
+ counted = TrueAlso applies to: 86-96
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/helpers/token_usage.py` around lines 38 - 56, The usage
aggregation in _add_usage_from_mapping currently returns True even when only
cache tokens are present, which prevents _merge_usage_maps/token_usage from
being applied and drops prompt/completion counts. Update _add_usage_from_mapping
so it only marks the mapping as handled after merging all complementary usage
fields, and make sure the logic in token_usage and the related helper path
around _merge_usage_maps combines cache-only and token-only maps before deciding
the call is counted.
|
|
||
| [tool.uv.sources] | ||
| cuga = { path = "../cuga-agent", editable = true } | ||
| appworld = { path = "benchmarks/appworld/appworld", editable = true } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Committed local-only appworld editable path contradicts documented policy.
benchmarks/appworld/README.md explicitly states the [tool.uv.sources] and [dependency-groups].appworld entries written by setup_appworld.sh are machine-local and must not be committed, since they point to a path (benchmarks/appworld/appworld) that only exists after a developer runs the setup script, and a pre-commit hook (scripts/check_no_local_appworld.sh) is supposed to block exactly this. This diff adds precisely those entries to the tracked pyproject.toml, which per the README's own warning "would re-break uv sync on fresh checkouts and in CI."
Please confirm whether this was committed intentionally (e.g., with --no-verify) or should be reverted before merge.
🔧 Suggested fix
[tool.uv.sources]
-appworld = { path = "benchmarks/appworld/appworld", editable = true }
+# appworld path added locally by setup_appworld.sh; do not commit. [dependency-groups]
-appworld = [
- "appworld",
-]
+# appworld group added locally by setup_appworld.sh; do not commit.As per path instructions in benchmarks/appworld/README.md: "don't commit the pyproject.toml / uv.lock diff. The script edits both pyproject.toml and uv.lock to point at a path that only exists on machines where the script has run."
Also applies to: 88-91
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyproject.toml` at line 44, Revert the committed machine-local appworld
entries from pyproject.toml: the [tool.uv.sources] appworld editable path and
the [dependency-groups].appworld entry should not be tracked because they are
only generated locally by setup_appworld.sh. Keep these changes out of the
committed configuration so fresh checkouts and CI do not point at
benchmarks/appworld/appworld, and ensure the tracked pyproject.toml matches the
documented policy enforced by scripts/check_no_local_appworld.sh.
Remove committed appworld editable source from pyproject/uv.lock so uv sync works without the submodule, split token metric helpers to avoid appworld imports in CI tests, and fix ruff/bandit findings in eval_status and tools.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TokenUsageCallbackthrough SDK and external eval paths, with unit tests for token rollup, harness integration, and tool-loop callbacks.smoke_external.sh) to verify LLM connectivity and agent tool loops without AppWorld servers, plus a live eval status dashboard (eval_status.sh) for compare runs.Cleanup included
generate_gpt52_report.py,backfill_tokens.py, etc.), and org-specific example URLs from docs..env); no secrets committed.status.htmldashboard template is tracked.Test plan
uv run pytest benchmarks/helpers/tests/test_token_usage.py benchmarks/helpers/tests/test_eval_status.py benchmarks/appworld/tests/test_token_harness.py benchmarks/appworld/tests/test_agent_adapters.py(31 passed)./benchmarks/appworld/smoke_external.sh --agents deepagentswith valid.env./benchmarks/appworld/compare.sh --eval-key test_challenge_easy --compare-agents --runs 1 --dry-run./benchmarks/appworld/eval.sh --agent deepagents --eval-key test_challenge_easySummary by CodeRabbit
New Features
Bug Fixes
Documentation