diff --git a/.claude/skills/merge-when-green/SKILL.md b/.claude/skills/merge-when-green/SKILL.md new file mode 100644 index 0000000..b4962a0 --- /dev/null +++ b/.claude/skills/merge-when-green/SKILL.md @@ -0,0 +1,42 @@ +--- +name: merge-when-green +description: Open a pull request for the current branch if needed, wait for CI to pass, then merge it with a merge commit and reset the working branch to main. Use when the user says "merge", "pr and merge", "pr merge", or "merge when green". Never merges while CI is red or still running. +--- + +# Merge when green + +Repo: `robdevops/finbot`. Base branch: `main`. Work branch: the one named in the session instructions (`claude/...`). +Use the GitHub MCP tools (`mcp__github__*`; load them with ToolSearch if they are not in the tool list). There is no `gh` CLI. + +## 1. Make sure there is a PR +1. `git status` and `git log origin/main..HEAD`. If there is uncommitted work, commit it first (attribution trailer from the session reminder) and push with `git push -u origin `. +2. `list_pull_requests` for `head: robdevops:`, `state: open`. If one exists, use it. If not, check `git log origin/main..HEAD` is non-empty ("No commits between main and branch" means a previous PR already merged: reset the branch to `origin/main` and tell the user, do not open an empty PR). +3. Otherwise `create_pull_request` (base `main`). Body: short bullet list of what changed and why, then the attribution lines from the session reminder (`🤖 Generated with [Claude Code](https://claude.com/claude-code)` and the session URL). Check for a PR template first (`.github/pull_request_template.md`); this repo has none today. + +## 2. Wait for CI +1. `pull_request_read` with `get_check_runs` (and `get_status` / `get` for `mergeable_state`) on the PR head. +2. Required: every check run `completed` with conclusion `success` (neutral/skipped are fine). The CI workflow has two matrix legs, `test (3.x)` and `test (3.13.5)`; both must pass. +3. No checks reported yet: wait a minute and look again. If still none after a few minutes, the workflow did not trigger. Say so and ask before merging; do not assume green. +4. Checks still running: call `subscribe_pr_activity` for the PR and end the turn. Do not poll with `sleep`. Merge when the CI event says green. As a backup, schedule one `send_later` check-in about an hour out. + +## 3. If CI is red +Do not merge. Read the failing job (`get_job_logs`), reproduce locally (`.venv/bin/python -m unittest discover -s tests`), fix the cause, and push. Rules: +- Never skip, disable or loosen a test (including the golden-image tolerance) just to get green. Refresh golden images only when the visual change was intended (`UPDATE_GOLDEN=1 python -m unittest tests.test_charts`). +- If the failure is not this PR's (reproduces on `main`, or an upstream outage), say what is failing and why, and ask the user whether to merge anyway. +- Re-check CI after each push; repeat until green. + +## 4. Merge +1. Confirm: PR open, not draft, `mergeable` true / no conflicts, all checks green. On a conflict, merge `origin/main` into the branch, resolve, run the tests, push, and go back to step 2. +2. `merge_pull_request` with `merge_method: merge` (merge commit, never squash or rebase unless the user asks). +3. Reset the work branch so follow-up work starts clean: + `git fetch origin main && git checkout -B origin/main && git push -q --force-with-lease -u origin ` + (this also clears the stop-hook's "unpushed commits" warning). +4. Unsubscribe from PR events if subscribed (`unsubscribe_pr_activity`). + +## 5. Report +One short message: PR link, merge commit short hash, which checks passed, and anything the user must do next (for example "restart the service to pick up the change"). Use markdown links for PRs (`[robdevops/finbot#NN](url)`), never bare `#NN`. + +## Never +- Merge with failing or missing checks without the user's explicit say-so. +- Push to `main` directly, force-push anything but the work branch, or rewrite history on someone else's branch. +- Merge a PR that has unresolved review requests from a human without telling the user. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20c61b8..c5e897d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,9 @@ on: push: branches: [main] pull_request: + schedule: + - cron: '23 19 * * 0' # weekly (Sunday, UTC): catches upstream package changes between commits + workflow_dispatch: # manual run from the Actions tab jobs: test: @@ -21,3 +24,25 @@ jobs: - run: pip install -r requirements.txt - run: python -m compileall -q bot.py *.py lib util - run: python -m unittest discover -s tests -v + + notify: + # Telegram alert when the weekly run fails. Needs repository secrets TELEGRAM_BOT_TOKEN and TELEGRAM_CHAT_ID. + needs: test + if: failure() && github.event_name == 'schedule' + runs-on: ubuntu-latest + steps: + - name: Send Telegram message + env: + TELEGRAM_BOT_TOKEN: ${{ secrets.TELEGRAM_BOT_TOKEN }} + TELEGRAM_CHAT_ID: ${{ secrets.TELEGRAM_CHAT_ID }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + if [ -z "$TELEGRAM_BOT_TOKEN" ] || [ -z "$TELEGRAM_CHAT_ID" ]; then + echo "::warning::TELEGRAM_BOT_TOKEN / TELEGRAM_CHAT_ID secrets are not set; no notification sent" + exit 0 + fi + curl -sS --fail-with-body "https://api.telegram.org/bot${TELEGRAM_BOT_TOKEN}/sendMessage" \ + --data-urlencode "chat_id=${TELEGRAM_CHAT_ID}" \ + --data-urlencode "disable_web_page_preview=true" \ + --data-urlencode "text=⚠️ ${GITHUB_REPOSITORY}: weekly tests failed + ${RUN_URL}" > /dev/null diff --git a/README.md b/README.md index cb76e6a..d3f5b91 100644 --- a/README.md +++ b/README.md @@ -492,8 +492,13 @@ Many objects are cached for just under one day by default. Cache is controlled b ``` cache = True cache_seconds = 82800 +performance_cache_seconds = 1800 ``` +`performance_cache_seconds` (default 30 minutes) is the cache lifetime of Sharesight performance reports. + +While `bot.py` is running (with `cache = True` and Sharesight configured), a background job keeps the Sharesight portfolio list (`cache_seconds`) and each portfolio's holdings report (1 day, `performance_cache_seconds` cache) refreshed at 90% of their cache lifetime, so chat requests do not wait on Sharesight. If a refresh fails, the previous cache keeps being served and the job retries after 5 minutes. + ### Hyperlinks These .env file options relate to hyperlinks: diff --git a/bot.py b/bot.py index 0ceda3d..8ad54df 100755 --- a/bot.py +++ b/bot.py @@ -10,6 +10,7 @@ from lib.config import * from lib import worker from lib import charts +from lib import sharesight from lib import webhook if config_telegramBotToken: from lib import telegram @@ -166,6 +167,8 @@ def git_version(): print(f'Opening socket on http://{config_ip}:{config_port}', file=sys.stderr) if config_telegramBotToken: threading.Thread(target=charts.refresh_keyboards, daemon=True).start() # update stale DM keyboards in the background + if config_cache and config_sharesight_auth.get('client_id'): + threading.Thread(target=sharesight.keep_warm, daemon=True).start() # keep the portfolio list and holdings report cached try: httpd.serve_forever() except OSError as e: diff --git a/lib/config.py b/lib/config.py index 33560c2..2f22885 100644 --- a/lib/config.py +++ b/lib/config.py @@ -27,6 +27,7 @@ config_cache_dir = os.getenv('cache_dir', 'var/cache').rstrip('/') config_var_dir = os.getenv('var_dir', 'var').rstrip('/') config_cache_seconds = int(os.getenv('cache_seconds', 43200 )) +config_performance_cache_seconds = int(os.getenv('performance_cache_seconds', 1800)) # Sharesight performance reports (30 minutes) config_country_code = os.getenv('country_code', 'AU') config_chunk_maxlines = int(os.getenv('chunk_maxlines', 20)) config_exclude_portfolios = () # default diff --git a/lib/sharesight.py b/lib/sharesight.py index 444f7aa..cb636f1 100644 --- a/lib/sharesight.py +++ b/lib/sharesight.py @@ -56,11 +56,18 @@ def get_token(): print("Got Sharesight token:", data['access_token'], file=sys.stderr) if debug else None return data['access_token'] -def get_portfolios(): +PORTFOLIOS_CACHE = "finbot_sharesight_portfolios.json" # ttl: config_cache_seconds +PERFORMANCE_TTL = config_performance_cache_seconds # seconds; the cache lifetime of a performance report + +def performance_cache_file(portfolio_id, days): + return "finbot_sharesight_performance_" + str(portfolio_id) + "_" + str(days) + '.json' + +def get_portfolios(refresh=False): + """refresh=True skips the cache read and fetches again (the cache is rewritten).""" portfolio_dict = {} cache = None - if config_cache: - cache_file = "finbot_sharesight_portfolios.json" + cache_file = PORTFOLIOS_CACHE + if config_cache and not refresh: cache = util.read_cache(cache_file, config_cache_seconds) if cache: data = cache @@ -158,11 +165,11 @@ def get_holdings_wrapper(): tickers = sorted(set(tickers)) return tickers -def get_performance(portfolio_id, days, config_cache_seconds=900): # cache for 15 minutes +def get_performance(portfolio_id, days, config_cache_seconds=PERFORMANCE_TTL, refresh=False): start_date = datetime.datetime.now() - datetime.timedelta(days=days) start_date = start_date.strftime('%Y-%m-%d') # 2023-04-25 - if config_cache: - cache_file = "finbot_sharesight_performance_" + str(portfolio_id) + "_" + str(days) + '.json' + cache_file = performance_cache_file(portfolio_id, days) + if config_cache and not refresh: cache = util.read_cache(cache_file, config_cache_seconds) if cache: return cache @@ -199,3 +206,32 @@ def get_performance_wrapper(days=config_past_days): print("Could not get performance for portfolio:", portfolio_id, file=sys.stderr) raise RuntimeError(f"Sharesight returned no performance for portfolio {portfolio_id}") return performance + +def warm_once(margin=0.9): + """Refresh the portfolio list and each portfolio's holdings report (days=0) once they reach `margin` of their cache lifetime, + so a request never has to wait on Sharesight. Returns the number of fetches made.""" + fetched = 0 + age = util.cache_age(PORTFOLIOS_CACHE) + if age is None or age >= margin * config_cache_seconds: + get_portfolios(refresh=True) + fetched += 1 + for portfolio_name, portfolio_id in get_portfolios().items(): + age = util.cache_age(performance_cache_file(portfolio_id, 0)) + if age is None or age >= margin * PERFORMANCE_TTL: + get_performance(portfolio_id, 0, refresh=True) + fetched += 1 + return fetched + +def keep_warm(poll=30, retry=300, stop=None): + """Background loop: keep the portfolio list and holdings report cached. Never raises; after a failure it waits `retry` seconds.""" + while not (stop and stop.is_set()): + wait = poll + try: + warm_once() + except Exception as e: + print("Sharesight cache refresh failed:", e, file=sys.stderr) + wait = retry + if stop: + stop.wait(wait) + else: + time.sleep(wait) diff --git a/lib/util.py b/lib/util.py index d6ebacb..9d71638 100644 --- a/lib/util.py +++ b/lib/util.py @@ -5,6 +5,7 @@ from pathlib import Path import re import sys +import time import numpy as np import matplotlib.dates as mdates from lib import sharesight @@ -303,6 +304,13 @@ def read_cache(cacheFile, maxSeconds=config_cache_seconds): print("cache miss:", cacheFile, file=sys.stderr) if debug else None return None +def cache_age(cacheFile): + """Seconds since the cache file was written, or None when it does not exist.""" + try: + return time.time() - os.path.getmtime(config_cache_dir + "/" + cacheFile) + except OSError: + return None + def json_write(filename, data, persist=False): if persist: filename = config_var_dir + "/" + filename diff --git a/requirements.txt b/requirements.txt index 94e3c36..e8515c5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ # Targets Python 3.13.5 (see .python-version). Lower bounds are the first releases with 3.13 support. gevent>=24.11.1 -matplotlib>=3.9.2 +matplotlib>=3.9.2,<4 numpy>=2.1.0,<3 pandas>=2.2.3,<3 # 3.x not yet tried python-dateutil>=2.9.0 diff --git a/tests/test_providers.py b/tests/test_providers.py index 4c02a3c..1c816ed 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -1,6 +1,6 @@ """Yahoo, Sharesight and Telegram parsing and failure handling, driven by the real API responses saved under doc/. HTTP is mocked at the requests layer, so the production parsing code runs unchanged.""" -import copy, json, os, unittest +import copy, datetime, json, os, unittest from unittest import mock from tests.common import * @@ -216,6 +216,95 @@ def test_performance_gives_up_after_repeated_busy_or_hard_errors(self): sharesight.get_performance(684141, 7) +class SharesightKeepWarm(ProviderCase): + """The background refresher keeps the portfolio list and the holdings report (days=0) cached ahead of their TTLs.""" + + def serve_sharesight(self): + portfolios = sample('sharesight', 'finbot_sharesight_portfolios.json') + performance = sample('sharesight', 'finbot_sharesight_performance_684141_0.json') + calls = [] + def get(url, **kwargs): + calls.append(url) + return Resp(performance if '/performance' in url else portfolios) + self.patch(sharesight.requests, 'get', get) + return calls + + def age(self, cache_file, seconds): + path = os.path.join(util.config_cache_dir, cache_file) + old = os.path.getmtime(path) - seconds + os.utime(path, (old, old)) + + def test_cold_cache_is_filled_then_left_alone(self): + calls = self.serve_sharesight() + self.assertEqual(sharesight.warm_once(), 3) # portfolio list + one holdings report per portfolio (2) + self.assertEqual(sum('/performance' in c for c in calls), 2) + self.assertTrue(all('start_date=' + datetime.date.today().isoformat() in c for c in calls if '/performance' in c)) # days=0 + self.assertEqual(sharesight.warm_once(), 0) + self.assertEqual(len(calls), 3) # nothing refetched while fresh + + def test_refreshes_before_the_ttl_runs_out(self): + calls = self.serve_sharesight() + sharesight.warm_once() + calls.clear() + self.age(sharesight.performance_cache_file(684141, 0), 0.95 * sharesight.PERFORMANCE_TTL) # 95% of its lifetime + self.assertEqual(sharesight.warm_once(), 1) + self.assertEqual(len(calls), 1) + self.assertIn('684141/performance', calls[0]) + calls.clear() + self.age(sharesight.PORTFOLIOS_CACHE, 0.95 * sharesight.config_cache_seconds) + self.assertEqual(sharesight.warm_once(), 1) + self.assertTrue(calls[0].endswith('/portfolios')) + + def test_a_request_never_waits_on_a_cache_the_job_has_warmed(self): + calls = self.serve_sharesight() + sharesight.warm_once() + calls.clear() + self.assertEqual(sharesight.get_portfolios(), {'Rob': 445825, 'RobSMSF': 684141}) + self.assertEqual(len(sharesight.get_performance(684141, 0)['report']['holdings']), 18) + self.assertEqual(calls, []) + + def test_holdings_wrapper_is_served_entirely_from_the_warmed_cache(self): + calls = self.serve_sharesight() + sharesight.warm_once() + calls.clear() + tickers = sharesight.get_holdings_wrapper() # get_portfolios() + get_holdings() per portfolio, no cache of its own + self.assertIn('ARM', tickers) + self.assertEqual(calls, []) + + def test_failed_refresh_keeps_the_old_cache(self): + self.serve_sharesight() + sharesight.warm_once() + self.age(sharesight.PORTFOLIOS_CACHE, 0.95 * sharesight.config_cache_seconds) + self.http(sharesight, ConnectionError('down')) + with self.assertRaises(RuntimeError): + sharesight.warm_once() + self.assertEqual(sharesight.get_portfolios(), {'Rob': 445825, 'RobSMSF': 684141}) # still served from the cache + + def test_loop_survives_failures_and_backs_off(self): + class Stop: + def __init__(self): self.waits = [] + def is_set(self): return len(self.waits) >= 2 + def wait(self, seconds): self.waits.append(seconds) + outcomes = [RuntimeError('boom'), 0] + def warm(): + outcome = outcomes.pop(0) + if isinstance(outcome, Exception): + raise outcome + return outcome + self.patch(sharesight, 'warm_once', warm) + stop = Stop() + sharesight.keep_warm(poll=30, retry=300, stop=stop) + self.assertEqual(stop.waits, [300, 30]) # slow retry after the failure, normal polling after success + + def test_refresh_flag_bypasses_a_fresh_cache(self): + calls = self.serve_sharesight() + sharesight.get_portfolios() + sharesight.get_portfolios() + self.assertEqual(len(calls), 1) + sharesight.get_portfolios(refresh=True) + self.assertEqual(len(calls), 2) + + class TelegramSending(FinbotCase): """webhook.write against failing endpoints. The real send path, not the base fixture's recorder."""