diff --git a/.claude/skills/merge-when-green/SKILL.md b/.claude/skills/merge-when-green/SKILL.md index 0a264970..442fa7b1 100644 --- a/.claude/skills/merge-when-green/SKILL.md +++ b/.claude/skills/merge-when-green/SKILL.md @@ -1,18 +1,20 @@ --- 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. This is the default for finished work in this repo: once tests pass, merge without waiting to be asked. Also use when the user says "merge", "pr", "pr and merge", or "merge when green". Never merges while CI is red or still running. +description: The ship pipeline, started manually by the owner. Open a pull request for the work branch if there is none, wait for CI, merge with a merge commit when green, then sync the branch (fast-forward from main, no history rewrite) and report. Use when the user sends "pr", "merge", "pr and merge", "ship", or "merge when green". Any one of those starts the whole pipeline; never ask for a second command. Never merges while CI is red or missing. --- -# Merge when green +# Ship: PR, CI, merge -**Default (the owner's standing instruction):** when a PR for this branch exists and every required check has passed, merge it. Do not wait for a separate "merge" message. Opening a PR for finished, committed work is likewise expected; do not leave work sitting on the branch. The "Never" rules below still apply: red or missing checks, conflicts, or unresolved human review requests mean stop and tell the user. +**Trigger:** the owner sends `pr`, `merge`, `pr and merge`, `ship`, or invokes this skill. Any one of them runs everything below to the end. Without a trigger, do not open a PR or merge; just commit and push to the work branch (see `CLAUDE.md`). + +**Once triggered:** open the PR if needed, wait for CI, merge when every required check has passed. No further confirmation is needed. The "Never" rules below still apply: red or missing checks, conflicts, or unresolved human review requests mean stop and tell the user. 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). +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 everything already shipped: sync the branch with `git merge origin/main`, tell the user, and 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 @@ -30,9 +32,9 @@ Do not merge. Read the failing job (`get_job_logs`), reproduce locally (`.venv/b ## 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). +3. Sync the work branch. It is the persistent dev branch the owner's test environment pulls, so never reset, rebase or force-push it: + `git fetch origin main && git merge origin/main && git push -u origin ` + (after a merge-commit merge this is a fast-forward or a trivial merge; it also clears the stop-hook's "unpushed commits" warning). 4. Unsubscribe from PR events if subscribed (`unsubscribe_pr_activity`). ## 5. Report @@ -40,5 +42,5 @@ One short message: PR link, merge commit short hash, which checks passed, and an ## 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. +- Push to `main` directly, or rewrite history on any branch (no reset, rebase, amend or force-push on the work branch either). - Merge a PR that has unresolved review requests from a human without telling the user. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..d35ce507 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,22 @@ +# finbot: how to work in this repo + +## Iterating (auto mode) +The owner chats back and forth; every message that asks for a change gets a change. +1. Make the change, run the tests (`.venv/bin/python -m unittest discover -s tests`), fix what breaks. +2. Commit (with the attribution trailer) and **push to the work branch** (`git push -u origin `). Do this every iteration, so the branch always holds the latest working state. +3. Reply briefly: what changed, test result, and the pushed commit's short hash. +4. **Do not open a PR, and do not merge, until the owner triggers the pipeline.** + +## The work branch is the dev branch +The work branch named in the session instructions (`claude/...`) is the persistent dev branch. The owner's test environment pulls it and restarts the service, so: +- **Never rewrite its history**: no reset to `main`, no rebase, no amend of pushed commits, no force-push. A force-push breaks `git pull` on the test environment. +- After a merge to `main`, bring the branch up to date with `git merge origin/main` (a fast-forward in the normal case) and a plain `git push`. The branch name and its history carry on. +- If the owner wants a fixed name such as `dev`, they will say so; only push to another branch with their explicit permission. + +## Shipping: the manual trigger +The owner starts the pipeline by sending **`pr`**, **`merge`**, or **`pr and merge`** (or `/merge-when-green`). Any one of them means the whole thing: open the PR if there is none, wait for CI, merge with a merge commit when green, sync the branch, report. Never ask them to send both. See `.claude/skills/merge-when-green/SKILL.md` for the steps and the stop conditions (red or missing CI, conflicts, unresolved human review). + +## Facts +- Python 3.13.5; tests are offline and need no network (`tests/`). CI runs them on the latest 3.x and 3.13.5. +- Production runs the Debian system packages; `requirements.txt` is for pip/uv users and CI. +- PR bodies end with the attribution lines from the session reminder; link PRs as `[robdevops/finbot#NN](url)`. diff --git a/lib/charts.py b/lib/charts.py index 3900ccef..c6c85bd3 100644 --- a/lib/charts.py +++ b/lib/charts.py @@ -10,18 +10,19 @@ from lib import yahoo from lib import reports -PERIODS = {'w': ('7D', 7), 'm': ('1M', 30), 'q': ('3M', 90), 'y': ('1Y', 365), 'x': ('Max', None)} -BUTTONS = {'h': 'wmqyx', 'p': 'wmqyx', 'c': 'wmqyx', 'f': 'wmqyx', 'l': 'wmqyx'} # chart kind -> buttons offered (h=history, p=price chart, c=compare, f=performance, l=price list) +PERIODS = {'d': ('1D', 1), 'w': ('7D', 7), 'm': ('1M', 30), 'q': ('3M', 90), 'y': ('1Y', 365), 'x': ('Max', None)} +BUTTONS = {'h': 'wmqyx', 'p': 'wmqyx', 'c': 'wmqyx', 'f': 'wmqyx', 'l': 'dwmqyx'} # chart kind -> buttons offered (h=history, p=price chart, c=compare, f=performance, l=price list) MAX_DAYS = 3650 # what Max means for the Sharesight-based reports HIGHLIGHT = {'w': '7D', 'm': '1M', 'q': '3M', 'y': '1Y', 'x': 'Max'} # history table row for each button MAX_CALLBACK_BYTES = 64 # Telegram limit on callback_data REF_FILE = 'finbot_chart_buttons.json' -def period_for_days(days): - """Button key for a day count, if it is one of the button periods.""" +def period_for_days(days, default=None): + """Button key for a day count, if it is one of the button periods; `default` when it is not (or days is None).""" for key, (_, d) in PERIODS.items(): if d and d == days: return key + return default def _ref(tickers): """Tickers as callback data: inline if it fits, else a short id persisted to disk.""" @@ -165,7 +166,10 @@ def build(kind, tickers, period, service='telegram'): if kind == 'l': # price list: tickers = [threshold, top] import price threshold, top = float(tickers[0]), int(tickers[1]) - payload, image = price.lambda_handler(threshold=threshold, service=service, interactive=True, days=days or MAX_DAYS, top=top or None, return_result=True) + if period == 'd': # today's moves: the command's default view, not a 1-day lookback + payload, image = price.lambda_handler(threshold=threshold, service=service, interactive=True, interday=True, top=top or None, return_result=True) + else: + payload, image = price.lambda_handler(threshold=threshold, service=service, interactive=True, days=days or MAX_DAYS, top=top or None, return_result=True) if not image: raise RuntimeError(payload[0] if payload else 'no price data') return '\n'.join(payload), image diff --git a/lib/worker.py b/lib/worker.py index bc8c7e14..f1a4af4d 100755 --- a/lib/worker.py +++ b/lib/worker.py @@ -64,7 +64,7 @@ def deliver(service, url, chat_id, payload, chart=None): def process_callback(service, callback_id, chat_id, message_id, data): """Button press on a chart message: rebuild it for the chosen period and edit it in place.""" parts = data.split('|', 3) - valid = len(parts) == 4 and parts[0] == 'c' and parts[1] in charts.BUTTONS and parts[2] in charts.PERIODS + valid = len(parts) == 4 and parts[0] == 'c' and parts[1] in charts.BUTTONS and parts[2] in charts.BUTTONS[parts[1]] # answering now shows Telegram's toast bubble ("Loading 3M…") while the new chart is built telegram.answerCallbackQuery(callback_id, f"Loading {charts.PERIODS[parts[2]][0]}…" if valid else None) if not valid: @@ -270,7 +270,7 @@ def alliterate(): typing.stop() elif m_performance: portfolio_select = None - days = config_past_days or 7 # performance needs a period; past_days = 0 means "today" elsewhere + days = charts.PERIODS['m'][1] # default view is the 1M button for arg in m_performance.groups()[:2]: # groups 2 and 3, allow arbitrary order if arg: try: diff --git a/price.py b/price.py index abccbe2a..518ba822 100755 --- a/price.py +++ b/price.py @@ -262,7 +262,7 @@ def short(ticker): if service == 'telegram' and specific_stock: markup = charts.keyboard('p', [specific_stock], charts.period_for_days(days)) elif service == 'telegram' and list_buttons: - markup = charts.keyboard('l', [f"{threshold:g}", str(top or 0)], charts.period_for_days(days)) + markup = charts.keyboard('l', [f"{threshold:g}", str(top or 0)], charts.period_for_days(days, 'd' if not days else None)) webhook.sendPhoto(chat_id, graph, caption, service, reply_markup=markup) else: webhook.payload_wrapper(service, url, payload, chat_id) @@ -272,7 +272,7 @@ def short(ticker): if service == "telegram": url = url + "sendMessage?chat_id=" + str(chat_id) if graph and service == 'telegram': - markup = charts.keyboard('l', [f"{threshold:g}", str(top or 0)], charts.period_for_days(days)) if list_buttons else None + markup = charts.keyboard('l', [f"{threshold:g}", str(top or 0)], charts.period_for_days(days, 'd' if not days else None)) if list_buttons else None webhook.sendPhoto(chat_id, graph, '\n'.join(payload), service, reply_markup=markup) continue webhook.payload_wrapper(service, url, payload, chat_id) diff --git a/tests/common.py b/tests/common.py index 6217fc1b..e44b5d3e 100644 --- a/tests/common.py +++ b/tests/common.py @@ -55,9 +55,10 @@ def setUp(self): for m in (performance, shorts, price, cal, trades, milestone, rating, worker, reminder): self.patch(m, 'webhooks', webhook.webhooks) self.sent = [] + self.markups = [] # reply_markup of each photo sent self.errors = [] self.patch(webhook, 'report_error', lambda e, *a, **k: self.errors.append(webhook.error_line(e, k.get('context')))) - self.patch(webhook, 'sendPhoto', lambda chat, img, cap, svc, **k: self.sent.append(('photo', len(img.read()), cap.split('\n')[0]))) + self.patch(webhook, 'sendPhoto', lambda chat, img, cap, svc, **k: (self.sent.append(('photo', len(img.read()), cap.split('\n')[0])), self.markups.append(k.get('reply_markup')))) self.patch(webhook, 'payload_wrapper', lambda svc, url, payload, *a, **k: self.sent.append(('text', payload)) or []) self.patch(util, 'get_holdings_and_watchlist', lambda: TICKERS) self.patch(yahoo, 'fetch', lambda ts: {t: market_entry(t) for t in ts}) @@ -81,6 +82,7 @@ def setUp(self): def command(self, text, chat='55'): """Run a chat message through the real command parser and handlers.""" self.sent.clear() + self.markups.clear() self.errors.clear() worker.process_request('telegram', chat, '@u', text, BOT, 'U', '1') self.assertEqual(self.errors, [], f'{text} crashed') # the worker reports crashes instead of raising diff --git a/tests/test_reports.py b/tests/test_reports.py index 10b41334..9acbd32b 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -99,6 +99,53 @@ def test_rating(self): self.assertTrue(self.sent) +class PeriodButtons(FinbotCase): + """Default views line up with the buttons: the highlighted button is the period actually shown.""" + + def buttons(self, command): + self.command(command) + self.assertTrue(self.markups and self.markups[0], f'{command} sent no buttons') + return [b['text'] for b in self.markups[0]['inline_keyboard'][0]] + + def test_performance_defaults_to_one_month(self): + self.assertEqual(self.buttons('.performance'), ['7D', '● 1M', '3M', '1Y', 'Max']) + self.assertIn('month', self.sent[0][2]) # "Performance over the past month", not 4 weeks + + def test_performance_period_argument_moves_the_marker(self): + self.assertEqual(self.buttons('.performance 7d'), ['● 7D', '1M', '3M', '1Y', 'Max']) + self.assertEqual(self.buttons('.performance 1y'), ['7D', '1M', '3M', '● 1Y', 'Max']) + + def test_price_list_has_a_1d_button_marked_by_default(self): + self.assertEqual(self.buttons('.price'), ['● 1D', '7D', '1M', '3M', '1Y', 'Max']) + self.assertEqual(self.buttons('.price top'), ['● 1D', '7D', '1M', '3M', '1Y', 'Max']) + self.assertEqual(self.buttons('.price 1m'), ['1D', '7D', '● 1M', '3M', '1Y', 'Max']) + + def test_other_charts_do_not_offer_1d(self): + for command in ('.performance', '.compare T1 T2'): + with self.subTest(command=command): + self.assertNotIn('1D', [b.lstrip('● ') for b in self.buttons(command)]) + + def press(self, data): + edits, toasts = [], [] + self.patch(webhook, 'editMessageMedia', lambda chat, msg, img, caption, markup=None: edits.append((caption.split('\n')[0], markup))) + self.patch(telegram, 'answerCallbackQuery', lambda cid, text=None: toasts.append(text)) + worker.process_callback('telegram', 'cb', '55', '9', data) + return edits, toasts + + def test_pressing_1d_rebuilds_todays_list_and_moves_the_marker(self): + edits, toasts = self.press('c|l|d|3,0') + self.assertEqual(toasts, ['Loading 1D…']) + self.assertEqual(self.errors, []) + self.assertEqual(len(edits), 1) + self.assertEqual([b['text'] for b in edits[0][1]['inline_keyboard'][0]][0], '● 1D') + + def test_a_button_a_chart_does_not_offer_is_ignored(self): + edits, toasts = self.press('c|f|d|') # performance has no 1D + self.assertEqual((edits, toasts), ([], [None])) + edits, toasts = self.press('c|l|zz|3,0') + self.assertEqual((edits, toasts), ([], [None])) + + class Podcasts(unittest.TestCase): def test_format_helpers(self): import podcasts