Portfolio Monitor ability#271
Conversation
Tracks a stock portfolio with real-time price monitoring via Finnhub (Alpha Vantage fallback). Fires proactive voice alerts on price thresholds, morning open summaries, and end-of-day wrap-ups. Supports add/remove/check/movers/alerts by voice with company name resolution.
Keep skill active when portfolio is empty — prompts to add inline
instead of exiting and handing off to TTT. Also fix cost_match
replace(',','') applied to match group, not raw input string.
Skip the user_response() call before _handle_add() — the final
transcription of the trigger phrase arrives at the exact moment
user_response() is called, so it gets consumed immediately, causing
_handle_add() to try resolving a non-ticker phrase via LLM. The LLM
call blocks long enough that the user's actual reply (Apple, NVDA, etc.)
falls outside the user_response() window and is intercepted by TTT.
Fix: call _handle_add('') directly — _resolve_ticker('') returns None
immediately (no LLM), so the skill asks 'Which stock?' right away with
the full response window available.
Also add _ADD_COMMAND_PHRASES to skip _resolve_ticker() for generic
add commands that never contain a ticker symbol.
…ruption
- does_match() now recognises "add Apple" / "add NVDA" via static TICKER_MAP
and ticker-pattern scan (no LLM) so the platform routes those utterances to
the skill after daemon startup
- _classify_intent() mirrors the same static-lookup rule so "add Apple" is
routed to ADD instead of falling through to PORTFOLIO
- Empty-portfolio branch in _handle_portfolio() now exits with instructions
("Say 'add Apple'...") instead of calling _handle_add(""), eliminating a
cold-start user_response() that never fired
- _handle_add() no-ticker branch exits with instructions instead of waiting on
a user_response() the daemon would swallow
- _handle_add() with-ticker path consolidated from two user_response() calls
(shares, then cost) to one combined prompt ("10 shares at 180"), halving the
number of calls that need to survive post-daemon routing
The platform routes utterances to skills via static HOTWORDS substring
matching at the initial invocation level — does_match() is only consulted
for within-skill user_response() routing, not for fresh invocations. So
'add Apple' was silently going to TTT because it was never in HOTWORDS.
- Extend HOTWORDS with {f"add {company}" for company in TICKER_MAP} so
'add Apple', 'add Tesla', etc. all trigger the skill correctly
- Rewrite _handle_add() as a zero-user_response() one-shot parser: extract
ticker, shares, and cost all from the trigger phrase; if incomplete, give
a concrete example and return
- Update empty-portfolio message to show the full one-shot syntax
Logs holding count on every wake cycle so we can tell whether the daemon reached its polling loop or hung silently inside _load_data().
…sleep
The platform extracts the HOTWORDS set literal at registration time, so the
runtime HOTWORDS |= {...} extension was invisible to its routing table —
'add Apple 10 shares at 180' never reached does_match() and went straight to
TTT. Fix: list all 'add [company]' phrases directly in the static literal so
the platform's pre-compiled routing picks them up.
Daemon was not blocked — it ran one tick correctly, found 0 holdings, then
slept POLL_MARKET_CLOSED (30 min). That's too long when the portfolio is
empty: the daemon should notice new stocks quickly. Add POLL_NO_HOLDINGS=30s
so the daemon re-checks every 30 seconds until the user adds their first stock.
…uting Platform uses exact phrase matching against HOTWORDS — 'add apple 10 shares at 180' never triggers because it's not in HOTWORDS as an exact entry. The exit-with-instructions approach was a dead end: telling the user to say a phrase that the platform can't route is worse than asking for it interactively. New flow: - 'add apple' (exact HOTWORD) triggers the skill - If trigger has both numbers (shares+cost) already: save immediately - Otherwise: ask ONE combined user_response() — daemon is sleeping by this point so resume_normal_flow() is not being called mid-wait, which should allow the response to be captured normally Update empty-portfolio and no-ticker messages to say 'add Apple' (the phrase that actually triggers) rather than the longer one-shot form.
…th design
Root cause established: the platform has a pre-compiled ASR-level hotword
detector built at ability registration — Python HOTWORDS changes are only
used inside does_match() for within-skill routing, not for initial invocation.
'add apple' (and all 'add [company]' variants we added) never fire the
detector. Only the original registered phrases work.
Changes:
- Remove all 'add [company]' entries from HOTWORDS — they were dead weight
- Rewrite _handle_add() with two clean paths:
Generic trigger ('add a stock'): ask WHO+HOW_MANY+AT_WHAT in one
question via user_response(), parse ticker+numbers from single reply
Specific trigger (company in phrase, e.g. future platform update):
try to parse numbers from trigger; if incomplete, ask shares+cost
via one user_response()
- Both paths share a single data load + duplicate check + save block
- Update empty portfolio message to reference 'add a stock' (the phrase
that actually triggers the skill)
Test: 'add a stock' -> 'Apple 10 shares at 180' -> watch for
'daemon tick — 1 holding(s)' to confirm user_response() works in 2nd
invocation when daemon is sleeping (not initializing).
- Replace all :.2f and :.1f price/pct formatting with integer-safe
equivalents so TTS reads cleanly (no space-inserted decimals)
- Smart pct: show "less than 1" when change_pct < 1%
- Portfolio view: while-loop follow-up so users can check multiple
stocks without re-invoking; unknown input gets a reprompt
- _speak_single_stock: use company name instead of ticker; add CTA
("say 'add a stock' to track it") for non-portfolio stocks
- Fix avg_cost display in add confirmation and duplicate warning
🔀 Branch Merge CheckPR direction: ✅ Passed — |
✅ Community PR Path Check — PassedAll changed files are inside the |
✅ Ability Validation Passed |
🔍 Lint Results✅
|
Bring the file in sync with upstream/dev so this PR has no diff outside the community/ folder — required by the path-check CI gate.
There was a problem hiding this comment.
Portfolio Monitor — Issues to Address
Tested end-to-end. The architecture (interactive skill + background daemon) is solid, but a handful of issues need addressing before merge. Listed roughly by impact:
-
One-shot handler + narrow
does_matchlets the agent LLM hallucinate ability behavior._run()handles a single intent then callsresume_normal_flow(), so follow-ups like"add stock","google","also add nvidia"don't re-passdoes_matchand the agent LLM invents fake confirmations ("Great choice! Google is now in your portfolio") while no state changes — daemon logs show0 holding(s)throughout. Fix: expandHOTWORDS, drop the"stock" not in texclusion in theaddregex, and make_handle_add/_handle_set_alertconversational with explicit-exit loops. Pattern reference:official/audius-music-dj. -
File I/O doesn't persist across triggers.
_save_datadoesdelete_file → write_filewith a bareexcept— any silent failure loses the file. Reproduced: added stocks, retriggered, "No stocks yet." Switch to Context Storage (get_single_key/create_key/update_key) — same APIaudius-music-djuses forfavoritesandglobal_context. Docs: capability-worker.md. -
Hardcoded API keys (
FINNHUB_KEY = "your_finnhub_api_key"etc.) with README telling users to editmain.py. Use the SDK pattern — single-key call returning the value directly:self.finnhub_key = self.capability_worker.get_api_keys("finnhub_api_key") or "" self.av_key = self.capability_worker.get_api_keys("alphavantage_api_key") or ""
Add an upfront check in
_run()that speaks a clear message when no keys are configured.
Docs reference: - https://docs.openhome.com/building-abilities/how-to-build#custom-api-keys-third-party-services -
Silent correctness bugs:
main.py:387—(position_pnl / position_cost * 100) if position_cost else 0is a dead expression; per-stock P&L % is never spoken.main.py:470—pos_pnl / pos_valueshould bepos_pnl / pos_cost. A stock that doubled reports "50%" instead of "100%"._handle_set_alertresets the per-ticker dict to{}before setting either threshold — setting drop and rise in separate sessions loses the earlier one. Usesetdefault(ticker, {})+is not Nonechecks.does_matchcalls_resolve_tickerwhich falls through to_resolve_ticker_llm→text_to_text_responsefires on every utterance, burning LLM quota.does_matchmust stay synchronous and cheap.
-
README claims that don't hold up:
- Daemon caps at 10 holdings (
MAX_API_CALLS_PER_POLL) — stocks past index 10 never refresh or alert. _handle_portfolioonly fetches when ticker is absent from cache — stale entries are never refreshed during a session.- EOD summary reads from the cache; misses closing-bell moves. Force a fresh fetch in the EOD window.
_handle_checkskips saving cache for non-portfolio stocks — re-hits API every time.
- Daemon caps at 10 holdings (
Let's bring this ability into full compliance with the SDK and voice UX requirements. Once fixed, we'd like to consider it for marketplace release — please update main.py and background.py accordingly to the best of your knowledge and best abilities practices.
- Switch file I/O to SDK Context Storage (get_single_key/create_key/
update_key) — eliminates silent data loss on delete→write failures
- API keys via get_api_keys() — no hardcoded secrets; upfront check in
_run() and watch_loop() speaks a setup message if key is missing
- Add _resolve_ticker_cheap() for does_match — static map + pattern
only, no LLM fired on every utterance; drop "stock not in t" exclusion
- _handle_add: conversational loop after each add ("want to add another?")
so follow-up stocks don't fall through to the agent LLM
- _handle_set_alert: use setdefault(ticker, {}) + is not None guards so
drop and rise thresholds set in separate sessions don't clobber each other
- _handle_portfolio: TTL-aware cache refresh (stale entries re-fetched,
not just missing ones); include position_pnl_pct in spoken stock lines
- _speak_single_stock: fix pos_pnl_pct denominator (pos_cost not pos_value)
- _handle_check: always save cache after fresh fetch, not only for
portfolio stocks
- _speak_eod_summary: force fresh quote fetch before computing EOD totals
- MAX_API_CALLS_PER_POLL: raised 10 → 50 (Finnhub allows 60/min free)
- Fix REMOVE intent false-positive: 'drop' now only routes to REMOVE with
explicit portfolio context; bare 'drop' phrases go to PORTFOLIO intent
- Fix TOCTOU race in _save_data (both files): try update_key first, fall
back to create_key on exception instead of check-then-act
- Fix chatty fetching in _handle_portfolio: pre-scan stale tickers and
announce "Fetching latest prices..." once instead of per-stock
- Fix _speak_single_stock: persist fresh quote to storage after fetch
- Fix _handle_add: validate shares/avg_cost > 0; cache resolved company
name to avoid double LLM call; map affirmative replies ("yes", "sure")
to generic trigger so the loop prompts for a new stock correctly
- Add follow-up loop to _handle_check: after each stock, ask if user
wants to check another rather than dropping back to idle
- Add follow-up loop to _handle_set_alert: after setting thresholds,
offer to set alerts for additional portfolio stocks
- Update README setup section: replace hardcoded-key instructions with
Settings → API Keys workflow
hassan1731996
force-pushed
the
feature/portfolio-monitor
branch
from
062edda to
13731a6
Compare
|
Hey @uzair401 - appreciate the thorough review, really helpful catching these before merge. Went through everything you flagged and pushed fixes for all of it: The LLM hallucination issue was the biggest one - Switched the whole persistence layer over to Context Storage ( API keys are now loaded at runtime via Fixed all four of the silent correctness bugs - the dead P&L expression, the On the daemon side: EOD summary now forces a fresh fetch before computing totals, Let me know if anything else needs attention. |
uzair401
left a comment
uzair401
left a comment
There was a problem hiding this comment.
Hello @hassan1731996,
This is a great ability and your contribution is much appreciated — I'd like to push it a bit further so it's marketplace-ready. A few directions I'd like us to take it:
Make it a one-stop place for stocks. Beyond just looking things up, it should also track and monitor the user's portfolio, provide live market insights, show current stock status, and do a basic day-over-day comparison (no need to go deeper than that for now).
Replace regex-based routing with an LLM intent router. Regex routing tends to be fragile for an ability this complex. Using the LLM as the intent router will route user requests much more reliably.
Restructure main.py to be more feature-centric. When the ability is triggered, the user should be able to check their portfolio, check current stock status, and view the previous day's comparison — all from one entry point.
Tighten intent handling. The check / move (or change) / delete flows need to work properly and consistently through the LLM router.
Once these are in, we'll do rigorous testing before submitting for marketplace approval. Thanks again for the work on this!
- Replace regex _classify_intent with LLM router: covers all intents including ambiguous natural-language requests reliably; cheap pre-filter retained only for CLEAR and REMOVE (unambiguous destructive actions) - Add COMPARE intent and _handle_compare: speaks each holding's move vs yesterday's close — direction, percent, and dollar change per stock - Restructure _handle_portfolio into a feature hub: opens with a tight two-line snapshot (value, day P&L, overall P&L), then prompts user to navigate — 'breakdown' for full detail, 'compare' for day-over-day, 'movers', or a stock name; full per-stock detail moved to _speak_portfolio_breakdown called on demand - Add compare phrases to HOTWORDS and README trigger phrases - Update README features section to reflect new intent router and COMPARE
|
Hey @uzair401 - pushed another round of changes based on your second review. LLM intent router is in. Day-over-day compare is a proper feature now. Saying "compare my stocks", "day over day", or "versus yesterday" triggers Feature-centric entry point - Let me know how testing goes. |
uzair401
left a comment
uzair401
left a comment
There was a problem hiding this comment.
Thanks @hassan1731996 for the quick turnaround on the requested changes. There are still a few improvements that need to be made, and I'll keep the PR open until those changes are in. I'll work through these, and once they're finalized by our reviewer and QA team, I'll share an update along with the revised code and updated README. After that, we'll proceed with marketplace approval. Thanks again for contributing to OpenHome!
Hey @uzair401 - glad the changes landed well. Happy to keep iterating if there's anything specific you want me to adjust on my end in the meantime. Looking forward to seeing what the QA pass surfaces - and appreciate you taking it the rest of the way to marketplace. Exciting milestone for this one. |
- New UPDATE intent: bought-more (weighted avg recalc), sold-some (share reduction or remove-if-zero), correct/overwrite — handles all position change flows in one conversational handler - New MARKET intent: live S&P 500, Nasdaq, Dow Jones pulse via same Finnhub feed - Hub navigation loop now uses LLM sub-router (_classify_hub_action) instead of keyword matching — consistent with top-level router - Hub prompt and snapshot updated to surface 'market' as a nav option - ADD flow: instead of dead-ending on existing stock, offers to update the position immediately via _handle_update - Chunked voice output for BREAKDOWN and COMPARE: 4 stocks per speak, paginate with "Want to hear the rest?" — prevents wall-of-text TTS - README: example conversation section, updated trigger phrases and feature descriptions for UPDATE and MARKET
|
Few more additions in this push: UPDATE intent - full position management in one flow: bought more shares (recalculates weighted average cost), sold some (reduces share count, or removes the stock entirely if you sell all of it with confirmation), and correct/overwrite if the numbers are just wrong. Reachable directly via voice ("I bought more Tesla", "sold some Apple") or from within the ADD flow when a stock already exists. MARKET intent - "how are the markets" pulls live S&P 500, Nasdaq, and Dow Jones from the same Finnhub feed. Works both as a standalone trigger and as a navigation option inside the portfolio hub. Hub navigation → LLM sub-router - unified the portfolio dashboard's follow-up loop with the same LLM routing approach used at the top level. Both now go through the same pattern so phrasing is handled consistently throughout. Chunked voice output - BREAKDOWN and COMPARE speak 4 stocks at a time and ask "Want to hear the rest?" before continuing. Keeps things natural for larger portfolios instead of reading everything out at once. README example conversation - added a full walkthrough: portfolio hub, market check, per-stock query, breakdown pagination, position update, and a background alert firing. Let me know if you have any other thoughts. |
2 participants
Portfolio Monitor
A passive background ability that tracks your stock portfolio in real time, fires proactive alerts when positions move beyond your thresholds, and delivers a full P&L breakdown on demand — all by voice.
What's in this PR
Core features
Interactive intents
PORTFOLIO— snapshot (value, day P&L, overall P&L) → hub navigationCHECK— specific stock with position P&L, follow-up loopCOMPARE— day-over-day vs yesterday's close, chunked at 4 stocksMOVERS— biggest gainer and loser in your portfolioMARKET— live S&P 500, Nasdaq, Dow Jones pulseADD— one-shot or prompted; offers UPDATE if stock already existsUPDATE— bought more (weighted avg cost recalc), sold some (share reduction or full remove), correct/overwriteSET_ALERT— drop/rise % thresholds per stock, loop for multipleREMOVE— with confirmationCLEAR— with confirmationVoice UX
Setup
What changed since initial review
COMPAREintent with day-over-day viewUPDATEintent (bought more / sold some / correct) — handles all position change flowsMARKETintent — S&P 500, Nasdaq, Dow via same Finnhub feed_classify_hub_action) for consistencyADDflow no longer dead-ends on existing stock — offers UPDATE insteaddoes_matchis fully synchronous — no LLM, no I/Oresume_normal_flowinfinallyblock, daemon startup onlyget_api_keys()at runtime, not hardcodedFiles
community/portfolio-monitor/main.py— foreground skillcommunity/portfolio-monitor/background.py— background daemoncommunity/portfolio-monitor/README.md— setup, trigger phrases, example conversation