ACE - Agentic Context Engineer #302
Conversation
Summary of ChangesHello @EduardGilM, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the Agentic Context Engineer (ACE) framework from Kayba, a significant enhancement designed to make TradingAgents more autonomous and robust. Instead of merely evaluating trade outcomes based on price fluctuations, ACE acts as a supervisor, guiding agents to reflect on the analytical consistency of their decision-making process. This involves a structured learning loop where market data, sentiment, news, and fundamental reports are evaluated against the agent's final decision. Critical feedback is generated, distilled into actionable strategies, and then seamlessly integrated into the Trader's prompts, allowing the agent to continuously learn from its analytical strengths and weaknesses and improve its reasoning over time. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR integrates the ACE (Agentic Context Engineer) framework from Kayba into the TradingAgents system to enable autonomous learning through analytical consistency reflection rather than purely price-based feedback.
Key changes include:
- Integration of the
ace-frameworkpackage for self-improving trading agents - New ACE learning module that evaluates logical consistency of trading decisions
- Dynamic injection of learned strategies into trader agent prompts
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| tradingagents/graph/trading_graph.py | Adds ACE engine initialization, learning hooks, and context injection methods for the trading graph |
| tradingagents/graph/setup.py | Updates trader node creation to accept ACE context parameter |
| tradingagents/default_config.py | Adds ACE configuration options from environment variables |
| tradingagents/agents/trader/trader.py | Modifies trader node to inject learned ACE strategies into prompts |
| tradingagents/ace/kayba_ace.py | Implements TradingACE class with learning, reflection, and skillbook management |
| tradingagents/ace/init.py | Exports ACE framework components and trading-specific implementations |
| requirements.txt | Adds ace-framework and python-dotenv dependencies |
| cli/main.py | Integrates ACE learning triggers and skillbook persistence into CLI workflow with UTF-8 encoding fixes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except: | ||
| try: | ||
| count = self.skillbook.stats().get('skills', 0) | ||
| except: |
There was a problem hiding this comment.
Using bare except clauses without specifying exception types is a bad practice as it catches all exceptions including system exits and keyboard interrupts. Specify the expected exception types (e.g., AttributeError, KeyError) to make error handling more precise and avoid masking unexpected issues.
| except: | |
| try: | |
| count = self.skillbook.stats().get('skills', 0) | |
| except: | |
| except Exception: | |
| try: | |
| count = self.skillbook.stats().get('skills', 0) | |
| except Exception: |
| except Exception as e: | ||
| print(f"Error in ACE learning: {e}") | ||
| import traceback | ||
| traceback.print_exc() |
There was a problem hiding this comment.
The exception is caught and logged but the error handling doesn't indicate what specific exceptions are expected. Consider catching specific exception types (e.g., IOError for file operations, AttributeError for missing methods) and provide more specific error messages to aid in debugging.
|
|
||
| # Get final state and decision | ||
| final_state = trace[-1] | ||
| graph.curr_state = final_state # Ensure curr_state is set for ACE |
There was a problem hiding this comment.
Missing space after the '#' in the inline comment. The comment should read '# Ensure curr_state is set for ACE' for consistency with Python style guidelines.
| graph.curr_state = final_state # Ensure curr_state is set for ACE | |
| graph.curr_state = final_state # Ensure curr_state is set for ACE |
| "ticker": context.split(" on ")[0], | ||
| "date": context.split(" on ")[1] if " on " in context else "Unknown" |
There was a problem hiding this comment.
The string parsing logic using split is fragile. If the context string doesn't contain ' on ', the code will still try to access index [1] which may not exist, potentially causing an IndexError. Consider using a more robust parsing approach or adding validation to handle cases where the expected format is not present.
| RiskDebateState, | ||
| ) | ||
| from tradingagents.dataflows.config import set_config | ||
| from tradingagents.ace import TradingACE, create_trading_ace |
There was a problem hiding this comment.
Duplicate import statement. This import is already present at line 47. Remove this duplicate import to keep the code clean.
|
|
||
| from __future__ import annotations | ||
|
|
||
| import os |
There was a problem hiding this comment.
Import of 'os' is not used.
| import os |
|
|
||
| import os | ||
| from pathlib import Path | ||
| from typing import Any, Dict, List, Optional |
There was a problem hiding this comment.
Import of 'List' is not used.
| from typing import Any, Dict, List, Optional | |
| from typing import Any, Dict, Optional |
| TaskEnvironment, | ||
| EnvironmentResult, | ||
| Skillbook, | ||
| Skill, |
There was a problem hiding this comment.
Import of 'Skill' is not used.
| Skill, |
| from tradingagents.ace import TradingACE, create_trading_ace | ||
|
|
||
| # Import the new abstract tool methods from agent_utils |
There was a problem hiding this comment.
Import of 'TradingACE' is not used.
| from tradingagents.ace import TradingACE, create_trading_ace | |
| # Import the new abstract tool methods from agent_utils | |
| # Import the new abstract tool methods from agent_utils |
| from .reflection import Reflector | ||
| from .signal_processing import SignalProcessor | ||
|
|
||
| from tradingagents.ace import TradingACE, create_trading_ace |
There was a problem hiding this comment.
Import of 'TradingACE' is not used.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates the ACE framework to enable autonomous learning for the trading agents, focusing on analytical consistency rather than just price outcomes. However, a security audit identified High-severity Path Traversal and Prompt Injection vulnerabilities. These stem from insufficient input validation on user-provided data, specifically the ticker symbol, which is used to construct file paths and LLM prompts, potentially allowing attackers to write files to arbitrary locations or manipulate LLM agent behavior. Furthermore, there are critical code duplication issues in tradingagents/graph/trading_graph.py and an opportunity to improve error handling in tradingagents/ace/kayba_ace.py by avoiding bare exception clauses.
| if self.ace_enabled: | ||
| self.ace_engine = create_trading_ace( | ||
| config=self.config, | ||
| skillbook_path=self.ace_skillbook_path, | ||
| ) | ||
| else: | ||
| self.ace_engine = None |
There was a problem hiding this comment.
This block of code, which initializes the ACE engine, is an exact duplicate of the preceding block (lines 109-115). This redundancy is likely a copy-paste error and should be removed.
| def _ace_learn_from_analysis(self): | ||
| """ | ||
| Trigger ACE learning based on the analytical consistency of all reports. | ||
| """ | ||
| if not self.ace_enabled or not self.ace_engine or not self.curr_state: | ||
| return | ||
|
|
||
| print(f"DEBUG: ACE analytical reflection triggered for {self.curr_state.get('company_of_interest')}") | ||
|
|
||
| reports = { | ||
| "ticker": self.curr_state.get("company_of_interest", "Unknown"), | ||
| "date": str(self.curr_state.get("trade_date", "Unknown")), | ||
| "market": self.curr_state.get("market_report", ""), | ||
| "sentiment": self.curr_state.get("sentiment_report", ""), | ||
| "news": self.curr_state.get("news_report", ""), | ||
| "fundamentals": self.curr_state.get("fundamentals_report", ""), | ||
| "plan": self.curr_state.get("investment_plan", ""), | ||
| } | ||
|
|
||
| decision = self.curr_state.get("final_trade_decision", "") | ||
| # Clean metadata tag if present | ||
| decision = re.sub(r"\[ACE_METADATA: .*\]", "", decision).strip() | ||
|
|
||
| self.ace_engine.learn_from_analysis( | ||
| reports=reports, | ||
| decision=decision | ||
| ) | ||
|
|
||
| def get_ace_context(self) -> str: | ||
| """Get ACE strategies context for injection into agent prompts.""" | ||
| if self.ace_enabled and self.ace_engine: | ||
| return self.ace_engine.get_skills_context() | ||
| return "" | ||
|
|
||
| def save_ace_skillbook(self, path: Optional[str] = None) -> str: | ||
| """Save the ACE skillbook to a file.""" | ||
| if self.ace_engine: | ||
| return self.ace_engine.save_skillbook(path) | ||
| return "" | ||
|
|
||
| def get_ace_stats(self) -> Dict[str, Any]: | ||
| """Get ACE learning statistics.""" | ||
| if self.ace_engine: | ||
| return self.ace_engine.get_stats() | ||
| return {} |
There was a problem hiding this comment.
The methods _ace_learn_from_analysis, get_ace_context, save_ace_skillbook, and get_ace_stats are duplicated. They are already defined starting at line 292. This entire block is a copy-paste error and must be removed to prevent runtime errors and maintain code sanity.
| results_dir = Path(config["results_dir"]) / selections["ticker"] / selections["analysis_date"] | ||
| results_dir.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
User input from the 'ticker' prompt is used to construct a file path without proper sanitization. An attacker can provide a malicious input containing '..' sequences (e.g., ../../tmp/exploit) to navigate outside the intended results directory and write files to arbitrary locations on the filesystem. The lack of input validation on selections["ticker"] allows for directory traversal. This vulnerability also affects tradingagents/graph/trading_graph.py at line 264. A single fix at the source of the input in cli/main.py will resolve all instances.
| context = { | ||
| "role": "user", | ||
| "content": f"Based on a comprehensive analysis by a team of analysts, here is an investment plan tailored for {company_name}. This plan incorporates insights from current technical market trends, macroeconomic indicators, and social media sentiment. Use this plan as a foundation for evaluating your next trading decision.\n\nProposed Investment Plan: {investment_plan}\n\nLeverage these insights to make an informed and strategic decision.", | ||
| "content": f"Based on a comprehensive analysis by a team of analysts, here is an investment plan tailored for {company_name}. This plan incorporates insights from current technical market trends, macroeconomic indicators, and social media sentiment. Use this plan as a foundation for evaluating your next trading decision.\n\nProposed Investment Plan: {investment_plan}\n\nLeverage these insights to make an informed and strategic decision.{ace_str}", |
There was a problem hiding this comment.
Untrusted user input (company_name, which originates from the ticker prompt) is directly embedded into the prompt for the trader LLM agent without sanitization. An attacker can provide a malicious ticker string to inject new instructions, potentially overriding the provided analysis and forcing a specific trading decision (e.g., BUY or SELL). This could lead to unauthorized trades and direct financial loss. Sanitize the company_name input by enforcing a strict allow-list of characters.
| question=( | ||
| f"Analyze the following multi-agent trading reports for {ticker} and " | ||
| "determine if the final decision is logically consistent with all data points." | ||
| ), |
There was a problem hiding this comment.
Untrusted user input from the 'ticker' prompt is directly embedded into a prompt for the reflector LLM agent without sanitization. An attacker can provide a malicious ticker string to inject new instructions into the prompt, potentially hijacking the reflection and learning process of the ACE framework. This could allow an attacker to manipulate the agent's learned skills. Sanitize the ticker input before using it in the LLM prompt by strictly validating its format.
| "max_recur_limit": 100, | ||
| # ACE (Agentic Context Engineering) settings | ||
| "ace_enabled": os.getenv("ACE_ENABLED", "true").lower() in {"true", "1", "yes", "on"}, | ||
| "ace_skillbook_path": os.getenv("ACE_SKILLBOOK_PATH", "results/ace_skillbook.json"), |
There was a problem hiding this comment.
The configuration ace_skillbook_path is loaded from the ACE_SKILLBOOK_PATH environment variable without any validation. This path is later used for file read/write operations. An attacker who can control the application's environment variables can set this to an arbitrary path to read or write files outside of the intended directory. Validate the path to ensure it is within an expected base directory.
| from .reflection import Reflector | ||
| from .signal_processing import SignalProcessor | ||
|
|
||
| from tradingagents.ace import TradingACE, create_trading_ace |
There was a problem hiding this comment.
The components TradingACE and create_trading_ace are imported twice in this file (at line 25 and again here). This second import is redundant and should be removed to improve code clarity and maintainability.
| try: | ||
| skills = self.skillbook.skills() | ||
| count = len(skills) | ||
| except: | ||
| try: | ||
| count = self.skillbook.stats().get('skills', 0) | ||
| except: | ||
| count = 0 | ||
|
|
There was a problem hiding this comment.
Using bare except: clauses is considered bad practice because it can catch and hide any exception, including system-exiting ones like SystemExit or KeyboardInterrupt. It's safer to catch specific exceptions or, at a minimum, Exception to avoid unintended consequences. Consider logging the exception for easier debugging.
| try: | |
| skills = self.skillbook.skills() | |
| count = len(skills) | |
| except: | |
| try: | |
| count = self.skillbook.stats().get('skills', 0) | |
| except: | |
| count = 0 | |
| try: | |
| skills = self.skillbook.skills() | |
| count = len(skills) | |
| except Exception: | |
| try: | |
| count = self.skillbook.stats().get('skills', 0) | |
| except Exception: | |
| count = 0 | |
|
Hi @EduardGilM have you run any backtrading with this new feature? If so could you please share the results? |
2 participants
ACE Integration (Kayba Framework)
This implementation integrates the official ACE framework from Kayba (
ace-framework) to allow TradingAgents to improve autonomously through reflection on their own Analytical Consistency.What is ACE?
ACE (Agentic Context Engineer) is a framework designed for AI agents to learn from their own experiences. In this repository, ACE acts as a "supervisor" that reviews the reasoning of analyst and trader agents to ensure that decisions are logical and consistent with the presented data.
Analytical Consistency vs. Price Delta
Unlike traditional learning systems that only look at whether the price went up or down (which can be noisy due to market volatility), this implementation uses an Analytical Consistency approach:
Final Decisionignores critical signals present in the reports (e.g., a very bearish fundamentals report but a buy decision).Reflectorgenerates constructive criticism on the logical flow of the analysis.SkillManagerdistills this criticism into "learned strategies" that are saved in theSkillbook.Installation
It is necessary to install the ACE framework:
Configuration (.env)
Configure ACE's behavior using the following environment variables:
Technical Details of the Implementation
1. Learning Engine (
kayba_ace.py)It uses
OnlineACEto manage the lifecycle. It implements a customTradingEnvironmentthat defines success not by PnL, but by logical rigor.2. Knowledge Persistence
The system uses
skillbook.apply_update()to ensure that new lessons are integrated immediately into the agent's memory without needing to restart the process.3. Knowledge Injection (
trader.py)The learned lessons are dynamically injected into the
Trader's prompt under theLearned Trading Strategies (ACE)section. This allows the agent to "remember" past errors in real time.4. Windows Compatibility (UTF-8)
The use of
utf-8encoding has been forced in all ACE logs and files to avoidUnicodeEncodeErrorerrors with special characters (such as mathematical symbols or technical emojis) in Windows environments.Usage
CLI Mode
When running
python -m cli.main, the system will automatically activate learning after each analysis. You will see logs indicating:DEBUG: ACE analytical reflection triggered for [TICKER]Automatic Mode (Scheduler)
The
scheduler.pyis fully integrated. After processing each ticker in the list, the bot will reflect on its analysis and save progress in theace_skillbook.json.Skillbook Structure
The generated file is a JSON that contains: