Add multi-agent portfolio rebalancer with semantic memory#438
Open
akash-kumar5 wants to merge 12 commits intoGetBindu:mainfrom
Open
Add multi-agent portfolio rebalancer with semantic memory#438akash-kumar5 wants to merge 12 commits intoGetBindu:mainfrom
akash-kumar5 wants to merge 12 commits intoGetBindu:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds two example agents: an Invoice Agent providing an in-memory X402-style invoice lifecycle and a Portfolio Rebalancer Agent implementing a multi-agent pipeline (Analyst → Pricer → Risk → Rebalancer) with semantic memory, configuration, skill manifests, and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Orchestrator as "Portfolio Rebalancer"
participant Analyst as "Analyst Agent"
participant Pricer as "Pricer Agent"
participant Risk as "Risk Agent"
participant Rebalancer as "Rebalancer Agent"
participant Memory as "Semantic Memory"
Client->>Orchestrator: send messages with portfolio & target
Orchestrator->>Analyst: portfolio + target
Analyst-->>Orchestrator: drift results
Orchestrator->>Pricer: symbols list
Pricer-->>Orchestrator: prices (API or fallback)
Orchestrator->>Risk: drift results
Risk-->>Orchestrator: risk scores + portfolio risk
Orchestrator->>Rebalancer: drift + prices + risk + memory + portfolio_value
Rebalancer-->>Orchestrator: trade plan + memory_update
Orchestrator->>Memory: update(memory_update)
Memory-->>Orchestrator: updated memory
Orchestrator-->>Client: actions + summary + timestamp + memory
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 13
🧹 Nitpick comments (5)
examples/invoice-agent/invoice_agent.py (2)
108-160: Inconsistent error response formats.Some error cases return strings (lines 113, 156) while others return structured dicts (lines 120, 140-143, 160). This inconsistency can cause issues for clients expecting a uniform response schema.
♻️ Proposed fix for consistent error responses
if not user_messages: - return "No user message found" + return {"error": "bad_request", "message": "No user message found"} raw = user_messages[-1].get("parts", [{}])[0].get("text", "{}")- return "Unknown request type" + return {"error": "bad_request", "message": "Unknown request type"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/invoice-agent/invoice_agent.py` around lines 108 - 160, The handler currently returns inconsistent error types (plain strings vs dicts); update handler to always return structured dicts for errors: replace the "No user message found" and "Unknown request type" string returns with standardized error dicts (e.g., {"error":"bad_request","message":"No user message found"} and {"error":"bad_request","message":"Unknown request type"}) and ensure the top-level exception handler (logger.exception in handler) matches this schema (e.g., {"error":"internal_error","message":"Internal Server Error"}), keeping existing structured returns for json decode and not-found cases unchanged so clients always receive a uniform JSON object.
70-85: Consider validatingtx_hashbefore marking as paid.The function accepts any
tx_hashvalue includingNone(when caller doesn't provide it). StoringNoneastx_hashwhile marking status as"paid"could cause confusion or issues for consumers expecting a valid transaction hash on paid invoices.🛡️ Proposed validation
def verify_payment(invoice_id, tx_hash): invoice = get_invoice_by_id(invoice_id) if not invoice: return {"verified": False, "reason": "Invoice not found"} + if not tx_hash: + return {"verified": False, "reason": "tx_hash is required"} + # mock verification invoice["status"] = "paid" invoice["tx_hash"] = tx_hash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/invoice-agent/invoice_agent.py` around lines 70 - 85, The verify_payment function currently accepts any tx_hash (including None) and then sets invoice["status"]="paid" and invoice["tx_hash"]=tx_hash; update verify_payment to validate the tx_hash before marking as paid: fetch the invoice with get_invoice_by_id, if invoice missing return as before, then check tx_hash is present and matches your expected format (e.g., non-empty string or regex for tx hash); if invalid, return {"verified": False, "reason": "Invalid or missing tx_hash"} and do not call save_invoice; only set invoice["status"]="paid", invoice["tx_hash"]=tx_hash and call save_invoice when tx_hash passes validation.examples/invoice-agent/README.md (1)
1-2: Mixed Markdown heading styles.Using both
#and====underline is redundant. Choose one style for consistency.📝 Proposed fix
# Invoice Agent with X402 Payment Flow -====================================🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/invoice-agent/README.md` around lines 1 - 2, The README uses mixed heading styles for "Invoice Agent with X402 Payment Flow" (a ATX '#' heading plus an underline-style setext '===='); make them consistent by removing the underline and keeping the single-line '#' heading (or alternatively remove the '#' and keep the underline), ensuring the heading text "Invoice Agent with X402 Payment Flow" appears once with a single Markdown heading style.examples/portfolio-rebalancer/agents/pricer.py (1)
34-34: Use shared timeout config instead of hardcoding.Line 34 duplicates timeout config and can drift from environment-driven behavior.
Suggested fix
import requests +from shared.config import REQUEST_TIMEOUT @@ - response = requests.get(url, params=params, timeout=5) + response = requests.get(url, params=params, timeout=REQUEST_TIMEOUT)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/portfolio-rebalancer/agents/pricer.py` at line 34, The requests.get call currently hardcodes timeout=5 (see the expression "response = requests.get(url, params=params, timeout=5)"); replace that hardcoded literal with the shared timeout configuration used by the project (for example use a module-level REQUEST_TIMEOUT or settings.REQUEST_TIMEOUT from the existing config module), import that config symbol in pricer.py if needed, and use it in the call (e.g., timeout=REQUEST_TIMEOUT) with a sensible fallback default if the config value is absent.examples/portfolio-rebalancer/shared/memory.py (1)
21-25: Shallow overwrite is fragile for nested memory fields.
self.store[key] = valuereplaces nested objects entirely (for example, partialpreferencesupdates can drop existing keys). Consider merging known nested dicts.Proposed fix
for key, value in new_data.items(): if key == "history": self.store["history"].append(value) + elif key == "preferences" and isinstance(value, dict): + self.store.setdefault("preferences", {}).update(value) else: self.store[key] = value🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/portfolio-rebalancer/shared/memory.py` around lines 21 - 25, The current update loop in memory.py does a shallow overwrite (self.store[key] = value) which clobbers nested dicts like preferences; change the logic in the method that iterates new_data.items() so that for known nested dictionary fields (e.g., "preferences", maybe others) you merge instead of replace—if both existing self.store[key] and value are dicts, perform a shallow or recursive merge (preserving existing keys and updating/adding from value), otherwise fall back to replacement; keep the special-case for "history" appending as-is and ensure type checks handle non-dict existing values safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/invoice-agent/.env.example`:
- Around line 1-3: Add the missing X402_NETWORK environment variable to the
example env file so users can configure the network used by the invoice agent;
update examples/invoice-agent/.env.example to include a line like
X402_NETWORK=base-sepolia (or another default) and mention it aligns with the
os.getenv("X402_NETWORK", "base-sepolia") call referenced in invoice_agent.py so
readers know the variable exists and can override the default.
In `@examples/invoice-agent/README.md`:
- Around line 60-81: The example JSON in README.md incorrectly shows
payment_header as a string; update the example to match the actual
implementation in invoice_agent.py (payment_header returned as an object) by
replacing the string value with an object containing keys amount (stringified
invoice["total"]), token (invoice.get("currency","USDC")), network
(os.getenv("X402_NETWORK","base-sepolia")), and pay_to_address
(invoice["recipient_wallet"]) so the README reflects the real response shape.
In `@examples/portfolio-rebalancer/.env.example`:
- Around line 1-2: The .env example is missing REQUEST_TIMEOUT which
shared/config.py reads; update examples/portfolio-rebalancer/.env.example to
include a REQUEST_TIMEOUT entry (with a sensible default value and unit
consistent with shared/config.py, e.g., milliseconds or seconds) so users can
tune HTTP timeouts; reference the REQUEST_TIMEOUT variable and ensure the
example's formatting matches the other entries (e.g., KEY=VALUE).
In `@examples/portfolio-rebalancer/agents/pricer.py`:
- Around line 59-61: The current except Exception block around the pricing logic
swallows all errors and returns stale FALLBACK_PRICES for every symbol; change
it to catch and handle only expected errors (e.g., network/API errors like
requests.exceptions.RequestException, JSONDecodeError, KeyError, TypeError) in
the pricing function (lookup where symbols and FALLBACK_PRICES are used), log
the concrete error with context, and either re-raise unexpected exceptions or
fall back on a per-symbol basis (only use FALLBACK_PRICES.get(s, 1) for symbols
that actually failed to fetch) so API/data bugs are not masked for all symbols.
In `@examples/portfolio-rebalancer/agents/rebalancer.py`:
- Around line 12-19: The parameter prices in the run function signature is
unused; either remove it from the rebalancer contract or incorporate it into
sizing/reasoning. If prices are unnecessary, update the run(portfolio_id, drift,
portfolio_risk, memory, portfolio_value) signature (and all callers) to drop
prices; if pricing is required, use the prices dict inside run (e.g., when
computing trade sizes or validating target values) so the symbol prices is read
and affects the returned TradePlan; adjust any references to DriftResult,
TradePlan, or portfolio_value as needed.
In `@examples/portfolio-rebalancer/agents/risk.py`:
- Around line 32-43: The current if/elif chain causes the stable-asset check
(symbol in ["USDC","USDT"]) to short-circuit the concentration logic so stable
coins never get concentration score adjustments; update the logic in the
function that evaluates symbol and pct so the stable-asset classification is
handled separately from concentration checks (e.g., make the stable-asset check
its own if block or change the concentration branches to independent ifs) and
still allow the HIGH_CONCENTRATION and MOD_CONCENTRATION branches to run and
modify score, reason, and flags for USDC/USDT when pct exceeds thresholds
(references: symbol, pct, HIGH_CONCENTRATION, MOD_CONCENTRATION, score, reason,
flags).
In `@examples/portfolio-rebalancer/portfolio_rebalancer_agent.py`:
- Line 8: The global memory instance "memory = SemanticMemory()" leaks state
across requests; instead create or select a per-portfolio memory inside the
request handler (do not use the module-level "memory"). Move instantiation out
of module scope and partition by portfolio id (e.g., maintain a map from
portfolio_id to SemanticMemory or call SemanticMemory with a namespace/partition
arg if supported) so functions that read/update memory (the code around lines
where "memory" is used) always operate on memory_for_portfolio =
get_or_create_memory(portfolio_id).
- Line 68: The call to risk.run(drift) is unpacking two values but the first
(risk_scores) is never used; update the call site (risk.run) usage to only
capture the needed value by either assigning the unused value to _ or directly
assigning a single variable (e.g., portfolio_risk = risk.run(drift) if the
function returns one value or modify the caller to accept only the second
return), and remove the unused variable name risk_scores from the assignment to
eliminate the dead binding.
- Around line 48-60: The loop that builds Asset instances from raw_portfolio
accepts current_value as-is, which can be non-numeric and later break
Portfolio.total_value; update the validation in the asset-building block to
normalize and validate current_value (e.g., coerce to float or Decimal, reject
None/empty/non-numeric strings) before constructing Asset(symbol=...,
current_value=...), skip/record invalid entries and after the loop fail fast
(raise a clear ValueError or similar) if no valid assets were produced;
reference the raw_portfolio input, the Asset constructor usage, and Portfolio
construction so you place validation right where assets are appended and ensure
Portfolio.total_value sees only numeric values.
In `@examples/portfolio-rebalancer/README.md`:
- Line 6: Fix markdown lint issues by removing the extra leading space before
the "Architecture" and the other affected heading (ensure the headings start
with "##" with no extra space before the text), and update the phrasing
"decision-making" to use the preferred style (change to "decision making") in
the README so headings and hyphenation comply with the project's markdown/style
rules.
- Line 83: The README's project tree lists an incorrect filename; replace the
entry "portfolio_agent.py" with the correct documented entrypoint name
"portfolio_rebalancer_agent.py" in the structure tree so it matches the rest of
the docs and the actual code (verify references to portfolio_rebalancer_agent.py
elsewhere in the README to ensure consistency).
In `@examples/portfolio-rebalancer/shared/config.py`:
- Around line 5-6: REQUEST_TIMEOUT currently parses os.getenv("REQUEST_TIMEOUT",
5) with int(...) at import-time and can raise ValueError, causing module import
to fail; change parsing in the module by reading the raw env string (os.getenv),
then defensively converting to int inside a try/except or helper (e.g.,
parse_request_timeout) that falls back to a sane default and enforces min/max
bounds (e.g., between 1 and 300). Keep USE_FAKE_DATA logic as-is but use the
same pattern if you want consistency; update the REQUEST_TIMEOUT variable
assignment to use the safe parser so invalid or malicious env values no longer
crash import.
In `@examples/portfolio-rebalancer/shared/memory.py`:
- Around line 13-14: get() currently returns the live backing dict (self.store)
allowing external mutation; change get to return a snapshot (e.g., a shallow
copy via self.store.copy() or dict(self.store)) so callers cannot mutate
internal state directly and updates must go through update(); locate and modify
the Memory.get method to return the copied dict instead of self.store.
---
Nitpick comments:
In `@examples/invoice-agent/invoice_agent.py`:
- Around line 108-160: The handler currently returns inconsistent error types
(plain strings vs dicts); update handler to always return structured dicts for
errors: replace the "No user message found" and "Unknown request type" string
returns with standardized error dicts (e.g.,
{"error":"bad_request","message":"No user message found"} and
{"error":"bad_request","message":"Unknown request type"}) and ensure the
top-level exception handler (logger.exception in handler) matches this schema
(e.g., {"error":"internal_error","message":"Internal Server Error"}), keeping
existing structured returns for json decode and not-found cases unchanged so
clients always receive a uniform JSON object.
- Around line 70-85: The verify_payment function currently accepts any tx_hash
(including None) and then sets invoice["status"]="paid" and
invoice["tx_hash"]=tx_hash; update verify_payment to validate the tx_hash before
marking as paid: fetch the invoice with get_invoice_by_id, if invoice missing
return as before, then check tx_hash is present and matches your expected format
(e.g., non-empty string or regex for tx hash); if invalid, return {"verified":
False, "reason": "Invalid or missing tx_hash"} and do not call save_invoice;
only set invoice["status"]="paid", invoice["tx_hash"]=tx_hash and call
save_invoice when tx_hash passes validation.
In `@examples/invoice-agent/README.md`:
- Around line 1-2: The README uses mixed heading styles for "Invoice Agent with
X402 Payment Flow" (a ATX '#' heading plus an underline-style setext '====');
make them consistent by removing the underline and keeping the single-line '#'
heading (or alternatively remove the '#' and keep the underline), ensuring the
heading text "Invoice Agent with X402 Payment Flow" appears once with a single
Markdown heading style.
In `@examples/portfolio-rebalancer/agents/pricer.py`:
- Line 34: The requests.get call currently hardcodes timeout=5 (see the
expression "response = requests.get(url, params=params, timeout=5)"); replace
that hardcoded literal with the shared timeout configuration used by the project
(for example use a module-level REQUEST_TIMEOUT or settings.REQUEST_TIMEOUT from
the existing config module), import that config symbol in pricer.py if needed,
and use it in the call (e.g., timeout=REQUEST_TIMEOUT) with a sensible fallback
default if the config value is absent.
In `@examples/portfolio-rebalancer/shared/memory.py`:
- Around line 21-25: The current update loop in memory.py does a shallow
overwrite (self.store[key] = value) which clobbers nested dicts like
preferences; change the logic in the method that iterates new_data.items() so
that for known nested dictionary fields (e.g., "preferences", maybe others) you
merge instead of replace—if both existing self.store[key] and value are dicts,
perform a shallow or recursive merge (preserving existing keys and
updating/adding from value), otherwise fall back to replacement; keep the
special-case for "history" appending as-is and ensure type checks handle
non-dict existing values safely.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e1ecee1-1554-4ce6-97cd-d52cd4aeecfe
📒 Files selected for processing (15)
examples/invoice-agent/.env.exampleexamples/invoice-agent/README.mdexamples/invoice-agent/invoice_agent.pyexamples/invoice-agent/skills/invoice-agent-skill/skill.yamlexamples/portfolio-rebalancer/.env.exampleexamples/portfolio-rebalancer/README.mdexamples/portfolio-rebalancer/agents/analyst.pyexamples/portfolio-rebalancer/agents/pricer.pyexamples/portfolio-rebalancer/agents/rebalancer.pyexamples/portfolio-rebalancer/agents/risk.pyexamples/portfolio-rebalancer/portfolio_rebalancer_agent.pyexamples/portfolio-rebalancer/shared/config.pyexamples/portfolio-rebalancer/shared/memory.pyexamples/portfolio-rebalancer/shared/types.pyexamples/portfolio-rebalancer/skills/portfolio-rebalancer-agent-skill/skill.yaml
Comment on lines
+1
to
+3
| AGENT_WALLET_ADDRESS=0x_your_wallet_here | ||
| OPENROUTER_API_KEY=your_openrouter_api_key_here | ||
|
|
There was a problem hiding this comment.
Missing X402_NETWORK environment variable.
The invoice_agent.py handler references os.getenv("X402_NETWORK", "base-sepolia") at line 131, but this variable is not documented in the .env.example. While it has a default, users should know it's configurable.
📝 Proposed fix
AGENT_WALLET_ADDRESS=0x_your_wallet_here
OPENROUTER_API_KEY=your_openrouter_api_key_here
+X402_NETWORK=base-sepolia # optional, defaults to base-sepolia📝 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_WALLET_ADDRESS=0x_your_wallet_here | |
| OPENROUTER_API_KEY=your_openrouter_api_key_here | |
| AGENT_WALLET_ADDRESS=0x_your_wallet_here | |
| OPENROUTER_API_KEY=your_openrouter_api_key_here | |
| X402_NETWORK=base-sepolia # optional, defaults to base-sepolia | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/invoice-agent/.env.example` around lines 1 - 3, Add the missing
X402_NETWORK environment variable to the example env file so users can configure
the network used by the invoice agent; update
examples/invoice-agent/.env.example to include a line like
X402_NETWORK=base-sepolia (or another default) and mention it aligns with the
os.getenv("X402_NETWORK", "base-sepolia") call referenced in invoice_agent.py so
readers know the variable exists and can override the default.
Comment on lines
+60
to
+81
| ```json | ||
| { | ||
| "invoice_id": "inv_66ac3b32-6cb4-4588-bd06-f71160ba206c", | ||
| "total": 90, | ||
| "payment_header": "X402 0xE5bC3b8796432A70aC8450E2aaD54055d9e2DBb8:90" | ||
| } | ||
| { | ||
| "invoice": { | ||
| "id": "inv_66ac3b32-6cb4-4588-bd06-f71160ba206c", | ||
| "recipient": "acme@example.com", | ||
| "recipient_wallet": "0xE5bC3b8796432A70aC8450E2aaD54055d9e2DBb8", | ||
| "items": [ | ||
| { "description": "API", "quantity": 1, "unit_price": 50 }, | ||
| { "description": "Compute", "quantity": 2, "unit_price": 20 } | ||
| ], | ||
| "currency": "USDC", | ||
| "total": 90, | ||
| "status": "paid", | ||
| "tx_hash": "0xabc123" | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Example output doesn't match actual implementation.
The documented payment_header is a string ("X402 0xE5bC3b8796432A70aC8450E2aaD54055d9e2DBb8:90"), but the actual implementation in invoice_agent.py lines 128-133 returns an object:
"payment_header": {
"amount": str(invoice["total"]),
"token": invoice.get("currency", "USDC"),
"network": os.getenv("X402_NETWORK", "base-sepolia"),
"pay_to_address": invoice["recipient_wallet"],
}Update the example to reflect the actual response format.
📝 Proposed fix
## Example Output:
```json
{
"invoice_id": "inv_66ac3b32-6cb4-4588-bd06-f71160ba206c",
"total": 90,
- "payment_header": "X402 0xE5bC3b8796432A70aC8450E2aaD54055d9e2DBb8:90"
+ "payment_header": {
+ "amount": "90",
+ "token": "USDC",
+ "network": "base-sepolia",
+ "pay_to_address": "0xE5bC3b8796432A70aC8450E2aaD54055d9e2DBb8"
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/invoice-agent/README.md` around lines 60 - 81, The example JSON in
README.md incorrectly shows payment_header as a string; update the example to
match the actual implementation in invoice_agent.py (payment_header returned as
an object) by replacing the string value with an object containing keys amount
(stringified invoice["total"]), token (invoice.get("currency","USDC")), network
(os.getenv("X402_NETWORK","base-sepolia")), and pay_to_address
(invoice["recipient_wallet"]) so the README reflects the real response shape.
| A multi-agent system that analyzes portfolio allocation, evaluates risk, and generates rebalancing actions using semantic memory. | ||
|
|
||
|
|
||
| ## Architecture |
There was a problem hiding this comment.
Address markdown/style lint warnings in headings and phrasing.
Static hints are valid here: extra heading spaces (Lines 6 and 28) and hyphenation in “decision-making” (Line 21).
Suggested fix
-## Architecture
+## Architecture
@@
-- Risk-aware trade decision making
+- Risk-aware trade decision-making
@@
-## Run
+## RunAlso applies to: 21-21, 28-28
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 6-6: Multiple spaces after hash on atx style heading
(MD019, no-multiple-space-atx)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/portfolio-rebalancer/README.md` at line 6, Fix markdown lint issues
by removing the extra leading space before the "Architecture" and the other
affected heading (ensure the headings start with "##" with no extra space before
the text), and update the phrasing "decision-making" to use the preferred style
(change to "decision making") in the README so headings and hyphenation comply
with the project's markdown/style rules.
Paraschamoli
requested changes
Apr 6, 2026
Member
Paraschamoli
left a comment
Paraschamoli
left a comment
There was a problem hiding this comment.
hey @akash-kumar5 try to use any agentic framework also you have made the whole folder structure mess so try to keep it clean and simple.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
examples/portfolio-rebalancer/README.md (1)
6-6:⚠️ Potential issue | 🟡 MinorResolve markdownlint/style nits in headings and phrasing.
There are still extra spaces in ATX headings and wording inconsistency in “decision making”.
Suggested fix
-## Architecture +## Architecture @@ -- Risk-aware trade decision making +- Risk-aware trade decision-making @@ -## Run +## RunAlso applies to: 21-21, 28-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/portfolio-rebalancer/README.md` at line 6, Fix the markdown heading and phrasing nits by removing extra spaces in ATX headings (e.g., change "## Architecture" to "## Architecture") and normalize wording of “decision making” to the hyphenated form "decision-making" throughout the README; update the other occurrences referenced (the headings at the other two locations) so all ATX headings have a single space after the hashes and all instances use "decision-making" for consistency.examples/portfolio-rebalancer/portfolio_rebalancer_agent.py (2)
8-8:⚠️ Potential issue | 🟠 MajorModule-level semantic memory leaks state across portfolios/requests.
Shared global memory lets unrelated requests read/write the same history/preferences and is unsafe under concurrent handler execution.
Also applies to: 76-80, 98-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/portfolio-rebalancer/portfolio_rebalancer_agent.py` at line 8, The global module-level "memory = SemanticMemory()" leaks state across requests; instead instantiate a new SemanticMemory per agent/request—remove the module-level "memory" and create the memory inside the relevant constructor or request handler (e.g., inside PortfolioRebalancerAgent.__init__ or the per-request function that uses memory) so each portfolio gets its own SemanticMemory instance; update all references that read the global "memory" to use the instance attribute (self.memory) or the local variable passed into functions to avoid shared mutable state.
48-60:⚠️ Potential issue | 🟠 MajorNormalize
current_valueand fail fast when assets are invalid.
current_valueis accepted as-is; non-numeric values can break downstream arithmetic (portfolio.total_value).Proposed fix
assets = [] for a in raw_portfolio.get("assets", []): if "symbol" in a and "current_value" in a: + try: + current_value = float(a["current_value"]) + except (TypeError, ValueError): + continue assets.append( Asset( symbol=a["symbol"], - current_value=a["current_value"] + current_value=current_value ) ) + + if not assets: + return {"error": "No valid assets found in portfolio"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/portfolio-rebalancer/portfolio_rebalancer_agent.py` around lines 48 - 60, The asset parsing loop accepts current_value as-is which can be non-numeric and break portfolio.total_value; update the loop that builds Asset objects from raw_portfolio to validate and normalize current_value to a numeric type (e.g., float) before constructing Asset, and fail fast by raising a clear exception (ValueError) if an asset is missing "symbol" or "current_value" or if current_value cannot be converted to a number; specifically modify the block that iterates over raw_portfolio.get("assets", []) and the calls to Asset(...) so they perform the conversion/validation prior to creating the Asset instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/portfolio-rebalancer/portfolio_rebalancer_agent.py`:
- Around line 71-78: The call to rebalancer.run is passing an unsupported
keyword argument prices which causes a TypeError; update the call in
portfolio_rebalancer_agent.py to remove the prices keyword (or replace it with
the correct parameter name expected by agents/rebalancer.py::run), e.g., pass
prices data via the supported argument (check rebalancer.run's signature) or
preload prices into the rebalancer object/memory before calling run; ensure you
reference rebalancer.run and memory.get() (or the correct param name in
agents/rebalancer.py::run) when making the change so the call matches the
function signature.
- Around line 34-35: The code assumes messages[-1] is a dict and calls
last_msg.get(...), which will raise AttributeError if last_msg is not a dict;
update the logic around the last_msg and content assignment to validate the type
(e.g., by checking isinstance(last_msg, dict) or hasattr(last_msg, "get"))
before calling .get(); if the check fails, handle it by converting last_msg to a
safe default (e.g., content = {}) or raising/returning a controlled error with a
clear message so functions that reference last_msg (the messages list processing
and the content variable assignment) do not raise unhandled exceptions.
In `@examples/portfolio-rebalancer/README.md`:
- Around line 53-73: The README example output is inconsistent with the actual
return shape from portfolio_rebalancer_agent.py::handler (which returns an
envelope like
{"role":"assistant","content":{...,"timestamp":...,"memory":...}}); update the
README example to match the handler by wrapping the current payload inside the
handler envelope (i.e., top-level "role":"assistant" and "content" object that
contains portfolio_id, actions, summary plus timestamp and memory fields with
correct types/format), or alternatively change
portfolio_rebalancer_agent.py::handler to emit the flat payload shown in the
README—pick one approach and make the code and documentation consistent so
clients can rely on a single response schema.
In `@examples/portfolio-rebalancer/shared/config.py`:
- Around line 6-10: The agents are bypassing the config by hardcoding values;
update examples/portfolio-rebalancer/agents/rebalancer.py and agents/pricer.py
to import and use REBALANCE_FRACTION and REQUEST_TIMEOUT from shared.config
instead of literal 0.25 and timeout=5; also ensure REBALANCE_FRACTION in
shared/config.py safely falls back to a sensible default if the env var is
missing or invalid so importing agents won’t raise an exception.
- Line 6: REBALANCE_FRACTION is currently parsed directly with
float(os.getenv(...)) which will raise at import time for missing or non-numeric
values; change the logic in the module to read the raw env string
(os.getenv("REBALANCE_FRACTION")) into a variable, provide a sensible default or
None, then convert with a guarded conversion (try/except ValueError/TypeError)
and either set a default numeric value or raise a clear, explicit
RuntimeError/ValueError with context; update the REBALANCE_FRACTION binding to
use this validated value so importing the module won’t crash on bad/missing env
input.
---
Duplicate comments:
In `@examples/portfolio-rebalancer/portfolio_rebalancer_agent.py`:
- Line 8: The global module-level "memory = SemanticMemory()" leaks state across
requests; instead instantiate a new SemanticMemory per agent/request—remove the
module-level "memory" and create the memory inside the relevant constructor or
request handler (e.g., inside PortfolioRebalancerAgent.__init__ or the
per-request function that uses memory) so each portfolio gets its own
SemanticMemory instance; update all references that read the global "memory" to
use the instance attribute (self.memory) or the local variable passed into
functions to avoid shared mutable state.
- Around line 48-60: The asset parsing loop accepts current_value as-is which
can be non-numeric and break portfolio.total_value; update the loop that builds
Asset objects from raw_portfolio to validate and normalize current_value to a
numeric type (e.g., float) before constructing Asset, and fail fast by raising a
clear exception (ValueError) if an asset is missing "symbol" or "current_value"
or if current_value cannot be converted to a number; specifically modify the
block that iterates over raw_portfolio.get("assets", []) and the calls to
Asset(...) so they perform the conversion/validation prior to creating the Asset
instances.
In `@examples/portfolio-rebalancer/README.md`:
- Line 6: Fix the markdown heading and phrasing nits by removing extra spaces in
ATX headings (e.g., change "## Architecture" to "## Architecture") and
normalize wording of “decision making” to the hyphenated form "decision-making"
throughout the README; update the other occurrences referenced (the headings at
the other two locations) so all ATX headings have a single space after the
hashes and all instances use "decision-making" for consistency.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 841d5559-f1bd-43ff-bbc6-a6527fec66ce
📒 Files selected for processing (7)
examples/portfolio-rebalancer/.env.exampleexamples/portfolio-rebalancer/README.mdexamples/portfolio-rebalancer/agents/pricer.pyexamples/portfolio-rebalancer/agents/rebalancer.pyexamples/portfolio-rebalancer/portfolio_rebalancer_agent.pyexamples/portfolio-rebalancer/shared/config.pyexamples/portfolio-rebalancer/shared/memory.py
✅ Files skipped from review due to trivial changes (1)
- examples/portfolio-rebalancer/shared/memory.py
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/portfolio-rebalancer/agents/pricer.py
- examples/portfolio-rebalancer/agents/rebalancer.py
Comment on lines
+53
to
+73
| Output | ||
| ```json | ||
| { | ||
| "portfolio_id": "demo-1", | ||
| "actions": [ | ||
| { | ||
| "symbol": "BTC", | ||
| "action": "sell", | ||
| "amount_usd": 1500, | ||
| "reason": "BTC drift +10.0% (25% rebalance)" | ||
| }, | ||
| { | ||
| "symbol": "ETH", | ||
| "action": "buy", | ||
| "amount_usd": 1500, | ||
| "reason": "ETH drift -10.0% (25% rebalance)" | ||
| } | ||
| ], | ||
| "summary": "Rebalance by selling $1,500 and buying $1,500. Moderate portfolio risk" | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Example output schema is inconsistent with actual handler response.
portfolio_rebalancer_agent.py::handler returns {"role":"assistant","content":{...,"timestamp","memory"}}, but this example shows top-level portfolio_id/actions/summary only. This can break client integrations that follow the README literally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/portfolio-rebalancer/README.md` around lines 53 - 73, The README
example output is inconsistent with the actual return shape from
portfolio_rebalancer_agent.py::handler (which returns an envelope like
{"role":"assistant","content":{...,"timestamp":...,"memory":...}}); update the
README example to match the handler by wrapping the current payload inside the
handler envelope (i.e., top-level "role":"assistant" and "content" object that
contains portfolio_id, actions, summary plus timestamp and memory fields with
correct types/format), or alternatively change
portfolio_rebalancer_agent.py::handler to emit the flat payload shown in the
README—pick one approach and make the code and documentation consistent so
clients can rely on a single response schema.
Comment on lines
+6
to
+10
| REBALANCE_FRACTION = float(os.getenv("REBALANCE_FRACTION")) | ||
| try: | ||
| REQUEST_TIMEOUT = max(1, int(os.getenv("REQUEST_TIMEOUT", 5))) | ||
| except ValueError: | ||
| REQUEST_TIMEOUT=5 No newline at end of file |
There was a problem hiding this comment.
Config knobs are defined here but bypassed by agent implementations.
REBALANCE_FRACTION and REQUEST_TIMEOUT are exposed in this module, but examples/portfolio-rebalancer/agents/rebalancer.py and examples/portfolio-rebalancer/agents/pricer.py currently hardcode 0.25 and timeout=5, so env overrides are ineffective.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/portfolio-rebalancer/shared/config.py` around lines 6 - 10, The
agents are bypassing the config by hardcoding values; update
examples/portfolio-rebalancer/agents/rebalancer.py and agents/pricer.py to
import and use REBALANCE_FRACTION and REQUEST_TIMEOUT from shared.config instead
of literal 0.25 and timeout=5; also ensure REBALANCE_FRACTION in
shared/config.py safely falls back to a sensible default if the env var is
missing or invalid so importing agents won’t raise an exception.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/portfolio-rebalancer/portfolio_rebalancer_agent.py`:
- Around line 53-57: The Asset constructor is being passed target_pct but the
Asset dataclass only defines symbol and current_value, causing a TypeError; fix
by adding target_pct: float = 0.0 to the Asset dataclass definition (e.g., in
shared/types.py) so Asset accepts the parameter, or alternatively remove
target_pct from the Asset(...) call in portfolio_rebalancer_agent.py (refer to
the Asset dataclass and the assets.append(Asset(...)) call to locate the code).
- Around line 70-71: The call to pricer.run(symbols) assigns prices but the
result is never used; remove the unnecessary API call by deleting the symbols =
[a.symbol for a in portfolio.assets] and prices = pricer.run(symbols) lines (or
alternatively, if rebalancer.run expects prices, pass the fetched prices into
rebalancer.run). Locate the code around portfolio.assets, pricer.run and
rebalancer.run and either remove the dead pricer.run invocation or thread the
prices value into rebalancer.run so the fetched data is actually consumed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee792d27-1985-47cd-9710-a4ed66d04d5f
📒 Files selected for processing (4)
examples/portfolio-rebalancer/agents/pricer.pyexamples/portfolio-rebalancer/agents/rebalancer.pyexamples/portfolio-rebalancer/portfolio_rebalancer_agent.pyexamples/portfolio-rebalancer/shared/config.py
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/portfolio-rebalancer/agents/pricer.py
- examples/portfolio-rebalancer/agents/rebalancer.py
- examples/portfolio-rebalancer/shared/config.py
Comment on lines
+53
to
+57
| assets.append(Asset( | ||
| symbol=a["symbol"], | ||
| current_value=current_value, | ||
| target_pct=target.get(a["symbol"], 0) | ||
| )) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify Asset dataclass fields
ast-grep --pattern $'@dataclass
class Asset:
$$$'Repository: GetBindu/Bindu
Length of output: 314
Asset constructor does not accept target_pct parameter — runtime TypeError.
The Asset dataclass in shared/types.py only defines symbol: str and current_value: float. Passing target_pct at lines 53-57 will raise TypeError: Asset.__init__() got an unexpected keyword argument 'target_pct'.
Either add target_pct: float = 0.0 to the Asset dataclass or remove the parameter from the constructor call:
Option 1: Remove target_pct from Asset construction
assets.append(Asset(
symbol=a["symbol"],
- current_value=current_value,
- target_pct=target.get(a["symbol"], 0)
+ current_value=current_value
))Option 2: Add target_pct to Asset dataclass
`@dataclass`
class Asset:
symbol: str
current_value: float
target_pct: float = 0.0📝 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
| assets.append(Asset( | |
| symbol=a["symbol"], | |
| current_value=current_value, | |
| target_pct=target.get(a["symbol"], 0) | |
| )) | |
| assets.append(Asset( | |
| symbol=a["symbol"], | |
| current_value=current_value | |
| )) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/portfolio-rebalancer/portfolio_rebalancer_agent.py` around lines 53
- 57, The Asset constructor is being passed target_pct but the Asset dataclass
only defines symbol and current_value, causing a TypeError; fix by adding
target_pct: float = 0.0 to the Asset dataclass definition (e.g., in
shared/types.py) so Asset accepts the parameter, or alternatively remove
target_pct from the Asset(...) call in portfolio_rebalancer_agent.py (refer to
the Asset dataclass and the assets.append(Asset(...)) call to locate the code).
Comment on lines
+70
to
+71
| symbols = [a.symbol for a in portfolio.assets] | ||
| prices = pricer.run(symbols) |
There was a problem hiding this comment.
prices is fetched but never used.
The prices variable is assigned from pricer.run(symbols) but is not consumed anywhere after the prices keyword was removed from rebalancer.run(). This results in an unnecessary API call.
Either remove the pricer.run() call or use prices where needed.
Proposed fix (remove dead code)
drift = analyst.run(portfolio, target)
- symbols = [a.symbol for a in portfolio.assets]
- prices = pricer.run(symbols)
-
_, portfolio_risk = risk.run(drift)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/portfolio-rebalancer/portfolio_rebalancer_agent.py` around lines 70
- 71, The call to pricer.run(symbols) assigns prices but the result is never
used; remove the unnecessary API call by deleting the symbols = [a.symbol for a
in portfolio.assets] and prices = pricer.run(symbols) lines (or alternatively,
if rebalancer.run expects prices, pass the fetched prices into rebalancer.run).
Locate the code around portfolio.assets, pricer.run and rebalancer.run and
either remove the dead pricer.run invocation or thread the prices value into
rebalancer.run so the fetched data is actually consumed.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
examples/portfolio-rebalancer/portfolio_rebalancer_agent.py (1)
53-57: Consider coercingtarget_pctto float for consistency.
current_valueis explicitly coerced tofloat, buttarget_pctis used as-is from the input. If a client passes"25"(string) instead of25(number), downstream arithmetic could fail or produce unexpected results.🔧 Proposed fix
assets.append(Asset( symbol=a["symbol"], current_value=current_value, - target_pct=target.get(a["symbol"], 0) + target_pct=float(target.get(a["symbol"], 0)) ))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/portfolio-rebalancer/portfolio_rebalancer_agent.py` around lines 53 - 57, The Asset construction should coerce target_pct to a float to match current_value and avoid type errors: when building assets in the loop that appends Asset(...) (see assets.append and the target dict lookup target.get(a["symbol"], 0)), convert the retrieved target percentage to float (e.g., wrap the target.get(...) result with float(...) or defensively parse numeric-like strings) before passing it as target_pct so downstream arithmetic on Asset.target_pct is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@examples/portfolio-rebalancer/portfolio_rebalancer_agent.py`:
- Around line 53-57: The Asset construction should coerce target_pct to a float
to match current_value and avoid type errors: when building assets in the loop
that appends Asset(...) (see assets.append and the target dict lookup
target.get(a["symbol"], 0)), convert the retrieved target percentage to float
(e.g., wrap the target.get(...) result with float(...) or defensively parse
numeric-like strings) before passing it as target_pct so downstream arithmetic
on Asset.target_pct is consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 593b9abb-fbfb-4e11-8970-d33e6b043f4d
📒 Files selected for processing (2)
examples/portfolio-rebalancer/portfolio_rebalancer_agent.pyexamples/portfolio-rebalancer/shared/types.py
✅ Files skipped from review due to trivial changes (1)
- examples/portfolio-rebalancer/shared/types.py
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
2 participants
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
Adds a multi-agent portfolio rebalancer example demonstrating agent orchestration and semantic memory.
Features
Notes
Summary by CodeRabbit
New Features
Documentation