From e1725e0235597b239dcc157a346df41daf363ae2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 07:49:21 +0000 Subject: [PATCH 01/25] .pe takes trailing (or current) for the trailing P/E .pe [SYMBOL|top|bottom|negative] [trailing]: with trailing or current, beside top, bottom or negative in either order, the list is by the trailing P/E, of earnings reported, not the forward one. - Command gets options, a second word read beside its keyword (Args.option), so ".pe bottom current" has both; suggest offers it. - A trailing P/E is no analysts' estimate, so its list takes every stock and its chart's subtitle has the date alone; the forward P/E and the PEG still need min_analysts. - Usage lines, doc/commands.md and util/record_outputs.py's list follow. record_outputs: the usage text (.help, /start, .jobs add) and the two new steps differ. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- CLAUDE.md | 4 ++-- doc/commands.md | 6 +++--- lib/reports/rankings.py | 38 ++++++++++++++++++++++---------------- lib/worker.py | 36 ++++++++++++++++++++++-------------- tests/test_discord.py | 2 +- tests/test_reports.py | 27 +++++++++++++++++++++++++++ tests/test_telegram_api.py | 2 +- util/record_outputs.py | 3 ++- 8 files changed, 80 insertions(+), 38 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9296ff0..e0376ff 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,8 +84,8 @@ An open PR takes in every commit pushed to its branch, so a PR opened for one sm Discord's registered commands); `suggest(chat, text)` completes a slash command's arguments as they are typed (Discord's autocomplete), from the cache alone, offering what a command `takes` (tickers or portfolios). `read()` finds the command by its whole name, the longest that matches (any other first word that may be a ticker is - `.SYMBOL`, that word its one word), and `read_args()` its `Args`: a percent, a period and a keyword from either end, - the words between. `answered()` runs a message's command and a job's alike: more or fewer words than its `words` + `.SYMBOL`, that word its one word), and `read_args()` its `Args`: a percent, a period, a keyword and an option (`.pe`'s + trailing) from either end, the words between. `answered()` runs a message's command and a job's alike: more or fewer words than its `words` (`Command.fits`), or a handler's `Usage`, get its usage; an Answer is the reply (Nothing never in a job's run); a handler gets a ticker with `ticker(args)`. `Request(dest, user, bot_name)` (`request.reply(lines, chart, buttons)`); each command gets the typing notice while it works. `process_request(message)` answers a message diff --git a/doc/commands.md b/doc/commands.md index ae1fd22..d388fb9 100644 --- a/doc/commands.md +++ b/doc/commands.md @@ -42,7 +42,7 @@ Where: /jobs add reminder schedule "text" /marketcap [symbol|top|bottom] /movers [percent|top|bottom|session|premarket] [period] -/pe [symbol|top|bottom|negative] +/pe [symbol|top|bottom|negative] [trailing] /peg [symbol|top|bottom|negative] /performance [period] [portfolio] /price [symbol...] [period] @@ -260,11 +260,11 @@ Command: ### pe / peg ![Forward PE report in Slack](../img/finbot_fpe.png?raw=true "Forward PE report in Slack") -List securities by forward P/E ratio (`/pe`), or PEG ratio (`/peg`). Both are analysts' estimates (a forward P/E of earnings, a PEG of their growth), so their lists take only stocks covered by at least `min_analysts` analysts (7 by default; see `settings.conf`), as `/buy` and `/sell` do. +List securities by forward P/E ratio (`/pe`), or PEG ratio (`/peg`). Both are analysts' estimates (a forward P/E of earnings, a PEG of their growth), so their lists take only stocks covered by at least `min_analysts` analysts (7 by default; see `settings.conf`), as `/buy` and `/sell` do. Add `trailing` (or `current`) to `/pe`, beside `top`, `bottom` or `negative` in either order, for the trailing P/E, of the earnings reported: it is no estimate, so every stock counts. Command: ``` -/pe [SYMBOL|top|bottom|negative] +/pe [SYMBOL|top|bottom|negative] [trailing] /peg [SYMBOL|top|bottom|negative] ``` diff --git a/lib/reports/rankings.py b/lib/reports/rankings.py index ef9d3a8..aa6a8fb 100644 --- a/lib/reports/rankings.py +++ b/lib/reports/rankings.py @@ -92,7 +92,9 @@ def prepare_rating_payload(service, chat, actions, length=None, asked=True): xlim=(max(0.9, low - spread * 0.9), min(5.1, high + spread * 0.25)), red_rows=tuple(red)) return payload, chart -RATIOS = {'forward pe': 'price_to_earnings_forward', 'peg': 'price_to_earnings_peg'} +RATIOS = {'forward pe': 'price_to_earnings_forward', 'trailing pe': 'price_to_earnings_trailing', + 'peg': 'price_to_earnings_peg'} +ESTIMATES = ('forward pe', 'peg') # the ratios that are analysts' estimates: their lists need min_analysts of them def ratio_line(service, ticker, kind): """One stock's line for .pe (the forward P/E) or .peg SYMBOL (kind: forward pe or peg): its P/E, trailing and @@ -113,21 +115,23 @@ def ratio_line(service, ticker, kind): return f"{links.stock_line(ticker, data, service)} {figures}" def ratio_list(service, chat, kind, mode='top', length=LENGTH): - """(payload, chart): the `length` stocks a chat follows (by its key) by a ratio (kind: forward pe or peg), - lowest first (mode top), highest first (bottom), or the negative ones only, lowest first (negative). Both are - analysts' estimates (a forward P/E of earnings, a PEG of their growth), so a list takes only stocks that - min_analysts or more analysts cover, as .buy does.""" + """(payload, chart): the `length` stocks a chat follows (by its key) by a ratio (kind: forward pe, trailing pe or + peg), lowest first (mode top), highest first (bottom), or the negative ones only, lowest first (negative). A + forward P/E and a PEG are analysts' estimates (of earnings, of their growth), so their lists take only stocks that + min_analysts or more analysts cover, as .buy does; a trailing P/E is of earnings reported, and takes all.""" field = RATIOS[kind] market_data = yahoo.fetch(watchlist.with_holdings(chat)) - details = yahoo.fetch_details(market_data) # how many analysts cover each, and its PEG - if kind == 'peg': - market_data |= details - covered = {ticker for ticker, detail in details.items() - if detail.get('recommend_analysts', 0) >= config.settings.min_analysts} + covered = None # the tickers enough analysts cover, for the estimates + if kind in ESTIMATES: + details = yahoo.fetch_details(market_data) # how many analysts cover each, and its PEG + if kind == 'peg': + market_data |= details + covered = {ticker for ticker, detail in details.items() + if detail.get('recommend_analysts', 0) >= config.settings.min_analysts} rows = [] # (ratio, chart label, line), one sorted list for the caption and the chart for ticker, data in market_data.items(): ratio = data.get(field) - if ratio is None or ticker not in covered or (ratio < 0) != (mode == 'negative'): + if ratio is None or (covered is not None and ticker not in covered) or (ratio < 0) != (mode == 'negative'): continue # too few analysts; the negative ones, or none of them rows.append((ratio, formats.short_ticker(ticker), f"{links.stock_line(ticker, data, service)} {ratio}")) rows.sort(key=lambda row: row[0]) @@ -135,13 +139,15 @@ def ratio_list(service, chat, kind, mode='top', length=LENGTH): rows.reverse() rows = rows[:length] trail = f'negative {kind}' if mode == 'negative' else kind # what the heading calls the ratio - analysts = f"{config.settings.min_analysts}+ analysts" # in the heading, and in the chart's subtitle + analysts = f"{config.settings.min_analysts}+ analysts" if covered is not None else '' # in the heading and subtitle if not rows: - raise answer.Nothing(f"No tracked stocks with {analysts} have a {trail} ratio") + raise answer.Nothing(f"No tracked stocks with {analysts} have a {trail} ratio" if analysts else + f"No tracked stocks have a {trail} ratio") heading = f"{'Bottom' if mode == 'bottom' else 'Top'} {length} tracked stocks by {trail} ratio" - chart = render.Chart('bars', heading, f"{analysts}, {formats.date_short(formats.today(), True)}", - tuple((label, float(ratio)) for ratio, label, _ in rows), value_format='number') - return [service.bold(f"{heading} ({analysts})")] + [line for _, _, line in rows], chart + subtitle = ', '.join(filter(None, (analysts, formats.date_short(formats.today(), True)))) + chart = render.Chart('bars', heading, subtitle, tuple((label, float(ratio)) for ratio, label, _ in rows), + value_format='number') + return [service.bold(heading + (f" ({analysts})" if analysts else ''))] + [line for _, _, line in rows], chart def prepare_beta_payload(service, chat): """(payload, chart) of the stocks a chat follows (by its key) with a beta over 1.5 and a market cap under US$2B, diff --git a/lib/worker.py b/lib/worker.py index b135547..93d9397 100755 --- a/lib/worker.py +++ b/lib/worker.py @@ -319,12 +319,14 @@ def answered(request, command, args): @dataclasses.dataclass class Args: """What a command was given (read_args): its name as written, a percent and a period where it takes them, one of its - keywords, and the words left between, which say what it is about: tickers or a portfolio's name.""" + keywords and one of its options, and the words left between, which say what it is about: tickers or a portfolio's + name.""" word: str text: str = '' # the arguments as written percent: float = None days: int = None keyword: str = None + option: str = None words: list = dataclasses.field(default_factory=list) @property @@ -360,8 +362,8 @@ def read(message, bot_name): def read_args(command, word, text): """The Args of a command called by `word`: from either end of its arguments, a percent where it takes one (5, 5% or - 2.5), a period where it takes one (7d, 3m, ytd; a bare number where it takes no percent) and one of its keywords, in - any case; the words left between are what it is about.""" + 2.5), a period where it takes one (7d, 3m, ytd; a bare number where it takes no percent), one of its keywords and + one of its options, in any case; the words left between are what it is about.""" args = Args(word, text.strip()) def taken(word): """Whether the word is an option the command takes and has not had yet, which it then has.""" @@ -380,6 +382,9 @@ def taken(word): if args.keyword is None and word.lower() in command.keywords: args.keyword = command.keywords[word.lower()] return True + if args.option is None and word.lower() in command.options: + args.option = command.options[word.lower()] + return True return False words = text.split() while words: @@ -499,15 +504,16 @@ def on_marketcap(request, args): request.reply(rankings.marketcap_list(request.service, request.dest.key, bottom=args.keyword == 'bottom')) def on_ratio(kind): - """The handler of .pe (the forward P/E) or .peg (kind: forward pe or peg): the top (by default) or bottom 15 by the - ratio and its chart, the negative ones (where there can be any), or one ticker's ratio.""" + """The handler of .pe (the forward P/E, or the current one given the option) or .peg (kind: forward pe or peg): the + top (by default) or bottom 15 by the ratio and its chart, the negative ones (where there can be any), or one + ticker's ratio.""" def handler(request, args): - """.pe or .peg [SYMBOL|top|bottom|negative].""" + """.pe [SYMBOL|top|bottom|negative] [trailing|current] or .peg [SYMBOL|top|bottom|negative].""" if args.words: request.reply([rankings.ratio_line(request.service, ticker(args), kind)]) else: - mode = args.keyword or 'top' - request.reply(*rankings.ratio_list(request.service, request.dest.key, kind, mode)) + now = 'trailing pe' if args.option == 'trailing' else kind + request.reply(*rankings.ratio_list(request.service, request.dest.key, now, args.keyword or 'top')) return handler def on_beta(request, args): @@ -667,8 +673,8 @@ def on_profile(request, args): class Command: """A chat command: its names (typos too; a name may be several words, as 'forward pe'), the handler that answers it, its .help lines under its section's heading, the Telegram keyboard buttons that send it, and what its arguments - may hold besides words (read_args): a percent, a period, its keywords ({spelling: keyword}); then how many words it - takes, and what they are.""" + may hold besides words (read_args): a percent, a period, its keywords and its options ({spelling: keyword}); then + how many words it takes, and what they are.""" names: tuple handler: typing.Callable usage: tuple = () # its lines in .help; none keeps it out of .help @@ -677,6 +683,7 @@ class Command: percent: bool = False period: bool = False keywords: dict = dataclasses.field(default_factory=dict) + options: dict = dataclasses.field(default_factory=dict) # a word it may take beside a keyword (.pe top current) words: tuple = (0, None) # how many words it takes, at least and at most (None: any): others get its usage takes: str = '' # what its words are, for suggest and a job's example: 'ticker' or 'portfolio' (a name) job: bool = True # whether a job may run it (job_commands): not one that changes something @@ -730,7 +737,8 @@ def fits(self, args): keywords=TOP | {name: name for name in SESSIONS}, words=(0, 1)), # a word: session's or premarket's SYMBOL Command(BUY_SELL, on_rating, ('.buysell',), RISK, ('.buy & sell',)), # .buy & sell, .buy sell: both Command(('pe', 'forward pe', 'forwardpe', 'fpe'), on_ratio('forward pe'), - ('.pe [SYMBOL|top|bottom|negative]',), RISK, ('.pe',), keywords=NEGATIVE, words=(0, 1), + ('.pe [SYMBOL|top|bottom|negative] [trailing]',), RISK, ('.pe',), keywords=NEGATIVE, + options={'trailing': 'trailing', 'current': 'trailing'}, words=(0, 1), takes='ticker'), # the forward P/E: it was .forward pe Command(('peg',), on_ratio('peg'), ('.peg [SYMBOL|top|bottom|negative]',), RISK, ('.peg',), keywords=NEGATIVE, words=(0, 1), takes='ticker'), @@ -876,7 +884,7 @@ def job_commands(): def completes(command): """Whether suggest has words to offer for a command's arguments: its keywords, periods, tickers, portfolios, or those of /jobs.""" - return bool(command.handler is on_jobs or command.keywords or command.period or command.takes) + return bool(command.handler is on_jobs or command.keywords or command.options or command.period or command.takes) def suggest(chat, text): """What to suggest while a slash command's arguments are typed (Discord's autocomplete), from the message they make @@ -908,8 +916,8 @@ def argument_words(command, chat, given): """(word, shown) pairs that a command's arguments may hold next, for suggest, after those given (Args): the keywords its usage shows and periods, unless it has one already; portfolios; tickers (the chat's watchlist and the holdings, as cached).""" - words = [word for word in re.findall(r'[\w-]+', ' '.join(command.usage)) - if word in command.keywords and word not in command.names and given.keyword is None] + words = [word for word in re.findall(r'[\w-]+', ' '.join(command.usage)) if word not in command.names and ( + word in command.keywords and given.keyword is None or word in command.options and given.option is None)] if command.period and given.days is None: words += PERIOD_WORDS if command.takes: diff --git a/tests/test_discord.py b/tests/test_discord.py index 8c90237..242e0f4 100644 --- a/tests/test_discord.py +++ b/tests/test_discord.py @@ -360,7 +360,7 @@ def test_help_on_discord_lists_the_slash_commands(self): self.patch(transports.Transport, 'text', lambda dest, lines, buttons=None: sent.append(lines) or []) message = inbound.Message('discord', '77', '<@5>', '/help', '@99', '', '42/tok') worker.process_request(message) - self.assertIn('/pe [SYMBOL|top|bottom|negative]', sent[0]) + self.assertIn('/pe [SYMBOL|top|bottom|negative] [trailing]', sent[0]) self.assertIn('/stock SYMBOL', sent[0]) self.assertIn('.price, or <@99> price', sent[0]) self.env(discordOutgoingWebhook='https://example.com/discord') diff --git a/tests/test_reports.py b/tests/test_reports.py index cfdb72a..ae7ff2e 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -312,6 +312,33 @@ def listed(mode='top', kind='forward pe'): none() self.assertEqual(nothing.exception.lines, [said]) + def test_pe_takes_trailing_or_current_as_an_option_beside_its_other_words(self): + """.pe [top|bottom|negative] trailing (or current) lists the trailing P/E, of earnings reported, which needs no + analysts (a forward P/E does), with the option before or after the keyword; .peg takes it as a ticker.""" + analysts = {'T1': 7, 'T2': 2, 'T3': 12} + self.patch(watchlist, 'with_holdings', lambda chat: list(analysts)) + trailing = {'T1': 30, 'T2': 10, 'T3': -4} + self.patch(yahoo, 'fetch', lambda ts: {t: market_entry(t) | {'price_to_earnings_forward': 20, + 'price_to_earnings_trailing': trailing[t]} for t in ts}) + self.patch(yahoo, 'fetch_detail', lambda t, *a, **k: {t: market_entry(t) | {'recommend_analysts': analysts[t]}}) + charted, real = [], render.draw + self.patch(render, 'draw', lambda chart: charted.append(chart) or real(chart)) + def listed(text): + """(the chart's title, the tickers it shows) of the list a command gives.""" + charted.clear() + self.command(text) + return charted[0].title, [label for label, _ in charted[0].rows] + forward = listed('.pe') + self.assertEqual(forward[1], ['T1', 'T3']) # T2: too few analysts + for text in ('.pe trailing', '.pe current', '.pe trailing top', '.pe top current'): + with self.subTest(text=text): + title, tickers = listed(text) + self.assertEqual((tickers, 'trailing pe ratio' in title), (['T2', 'T1'], True)) # T3 is negative + self.assertEqual(listed('.pe bottom trailing')[1], ['T1', 'T2']) + self.assertEqual(listed('.pe negative current')[1], ['T3']) + self.assertEqual(rankings.ratio_list(TELEGRAM, 'telegram:55', 'trailing pe')[1].subtitle, + formats.date_short(formats.today(), True)) # no analysts in a trailing chart's subtitle + def test_market_caps_compare_in_us_dollars(self): """Yahoo gives a market cap in the stock's own currency: unconverted, a won market cap topped every list.""" stocks = {'K1.KS': ('KRW', 1.01e15), 'K2.KS': ('KRW', 5e11), 'U1': ('USD', 5.39e12), 'L1.L': ('GBp', 6.25e10), diff --git a/tests/test_telegram_api.py b/tests/test_telegram_api.py index 8e99171..36a2ec7 100644 --- a/tests/test_telegram_api.py +++ b/tests/test_telegram_api.py @@ -146,7 +146,7 @@ def call(method, **kwargs): self.assertEqual(calls, ['getMyCommands', 'setMyCommands', 'getMyCommands']) self.assertIn(f'telegram: {len(menu)} commands in the menu', log.getvalue()) names = [entry['command'] for entry in menu] - self.assertIn({'command': 'pe', 'description': 'pe [SYMBOL|top|bottom|negative]'}, menu) + self.assertIn({'command': 'pe', 'description': 'pe [SYMBOL|top|bottom|negative] [trailing]'}, menu) self.assertEqual([name in names for name in ('stock', 'jobs', 'profile', 'job')], [True, True, False, False]) self.assertTrue(all(re.fullmatch(r'[a-z0-9_]{1,32}', name) for name in names)) self.patch(telegram_common, 'call', mock.Mock(side_effect=RuntimeError('Telegram getMyCommands: Unauthorized'))) diff --git a/util/record_outputs.py b/util/record_outputs.py index ee0c6e0..583acd3 100755 --- a/util/record_outputs.py +++ b/util/record_outputs.py @@ -17,7 +17,8 @@ REPO = Path(__file__).resolve().parent.parent COMMANDS = ['.help', '/start', '.hello', '.thanks', '.who T1', '.holdings', '.holdings Rob', '.marketcap', '.marketcap bottom', '.marketcap T1', '.pe', '.pe bottom', '.pe T1', - '.pe negative', '.forward pe T1', '.peg', '.peg negative', '.peg T1', '.beta', '.buy', '.sell', '.buy & sell', + '.pe negative', '.pe trailing', '.pe bottom current', '.forward pe T1', '.peg', '.peg negative', '.peg T1', '.beta', + '.buy', '.sell', '.buy & sell', '.earnings', '.earnings T1', '.dividend', '.dividend T1', '.trades', '.trades 30d', '.trades 7d Nope', '.movers', '.movers 7d', '.movers 3', '.movers 3 7d', '.movers top', '.movers top 7d', '.price', '.price top', '.price T1', '.price T1 7d', '.session', From 7cbde4f4cba99f24a4fcd5a44bc337fe4cb1f555 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 09:02:46 +0000 Subject: [PATCH 02/25] /ratio replaces /pe and /peg: the ratio, then its end, as optional words .ratio [SYMBOL] [forward|current|peg] [top|bottom|negative|reverse] - The forward P/E by default; current (or trailing) is the trailing P/E, peg the PEG. The end is top (by default), bottom or reverse (the highest first) or negative, in either order with the ratio. - .pe, .forward pe, .fpe and .peg stay as names of it, .peg's the PEG, so typed commands and jobs stored under pe or peg run as before; .help, the slash command menus, the keyboard button and the Add menu list ratio alone. - A forward P/E and a PEG still need min_analysts analysts; a trailing P/E, of earnings reported, takes every stock. - Command.options (Args.option) is the second optional word that .pe trailing added; here it is the ratio, and the keyword the end. record_outputs: the usage text (.help, /start, .jobs add, the Add menu) and the three new .ratio steps differ; .pe and .peg runs are identical. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- CLAUDE.md | 8 ++++---- doc/commands.md | 10 ++++------ doc/configuration.md | 2 +- doc/discord.md | 2 +- doc/slack.md | 2 +- doc/telegram.md | 2 +- lib/reports/__init__.py | 2 +- lib/reports/about.py | 2 +- lib/reports/rankings.py | 2 +- lib/worker.py | 38 +++++++++++++++++--------------------- tests/test_discord.py | 4 ++-- tests/test_edges.py | 2 +- tests/test_inbound.py | 2 +- tests/test_reports.py | 33 +++++++++++++++++++++++---------- tests/test_schedule.py | 6 +++--- tests/test_scheduler.py | 13 +++++++------ tests/test_telegram_api.py | 3 ++- util/record_outputs.py | 3 ++- 18 files changed, 73 insertions(+), 63 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e0376ff..4faf17a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,8 +84,8 @@ An open PR takes in every commit pushed to its branch, so a PR opened for one sm Discord's registered commands); `suggest(chat, text)` completes a slash command's arguments as they are typed (Discord's autocomplete), from the cache alone, offering what a command `takes` (tickers or portfolios). `read()` finds the command by its whole name, the longest that matches (any other first word that may be a ticker is - `.SYMBOL`, that word its one word), and `read_args()` its `Args`: a percent, a period, a keyword and an option (`.pe`'s - trailing) from either end, the words between. `answered()` runs a message's command and a job's alike: more or fewer words than its `words` + `.SYMBOL`, that word its one word), and `read_args()` its `Args`: a percent, a period, a keyword and an option (`.ratio`'s + ratio, `.pe` and `.peg` its names) from either end, the words between. `answered()` runs a message's command and a job's alike: more or fewer words than its `words` (`Command.fits`), or a handler's `Usage`, get its usage; an Answer is the reply (Nothing never in a job's run); a handler gets a ticker with `ticker(args)`. `Request(dest, user, bot_name)` (`request.reply(lines, chart, buttons)`); each command gets the typing notice while it works. `process_request(message)` answers a message @@ -105,8 +105,8 @@ An open PR takes in every commit pushed to its branch, so a PR opened for one sm (`delivery.interactive(dest)`); a job's run, on schedule, Run now or `.jobs run`, posts nothing. Chat-only (`prepare_*`): `about` (.help, and the .jobs menu's screens: `jobs_screen`, `add_screen`, `next_screen`), `history` (.price, also called .compare and .history: one stock's line, or - two to eight rebased), `holdings` (.holdings .who .treemap .watchlist), `profile` (.SYMBOL), `rankings` (.marketcap .pe - .fpe .peg .beta .buy .sell). + two to eight rebased), `holdings` (.holdings .who .treemap .watchlist), `profile` (.SYMBOL), `rankings` (.marketcap .ratio + .beta .buy .sell). - `lib/delivery.py`: `Report(lines, chart, buttons)`, the destinations (the chat that asked, or a chat by its key with `chat(key)`, as its network's bot), `deliver(dest, report)` (a chart refused goes again with its heading alone as the caption, the lines as text after it), the one-line error report (to its chat; one that belongs to no chat only diff --git a/doc/commands.md b/doc/commands.md index d388fb9..6b75ac6 100644 --- a/doc/commands.md +++ b/doc/commands.md @@ -42,10 +42,9 @@ Where: /jobs add reminder schedule "text" /marketcap [symbol|top|bottom] /movers [percent|top|bottom|session|premarket] [period] -/pe [symbol|top|bottom|negative] [trailing] -/peg [symbol|top|bottom|negative] /performance [period] [portfolio] /price [symbol...] [period] +/ratio [symbol] [forward|current|peg] [top|bottom|negative|reverse] /shorts [percent|symbol] /trades [period] [portfolio] /treemap [portfolio] [sector|industry|country|currency|market] @@ -257,15 +256,14 @@ Command: /shorts [percent] ``` -### pe / peg +### ratio (pe, peg) ![Forward PE report in Slack](../img/finbot_fpe.png?raw=true "Forward PE report in Slack") -List securities by forward P/E ratio (`/pe`), or PEG ratio (`/peg`). Both are analysts' estimates (a forward P/E of earnings, a PEG of their growth), so their lists take only stocks covered by at least `min_analysts` analysts (7 by default; see `settings.conf`), as `/buy` and `/sell` do. Add `trailing` (or `current`) to `/pe`, beside `top`, `bottom` or `negative` in either order, for the trailing P/E, of the earnings reported: it is no estimate, so every stock counts. +List the stocks followed by a ratio: `/ratio` is the forward P/E, lowest first; `/ratio current` the trailing P/E (`current` or `trailing`), of the earnings reported; `/ratio peg` the PEG. Then, optionally, the end: `top` (the default), `bottom` or `reverse` for the highest first, or `negative`, in either order: `/ratio peg reverse`, `/ratio current negative`. A ticker (`/ratio T1`) gets its own figures. `/pe` and `/peg` still work, as `/ratio` and `/ratio peg`. A forward P/E and a PEG are analysts' estimates (of earnings, of their growth), so their lists take only stocks covered by at least `min_analysts` analysts (7 by default; see `settings.conf`), as `/buy` and `/sell` do; a trailing P/E is no estimate, so every stock counts. Command: ``` -/pe [SYMBOL|top|bottom|negative] [trailing] -/peg [SYMBOL|top|bottom|negative] +/ratio [SYMBOL] [forward|current|peg] [top|bottom|negative|reverse] ``` ### Price diff --git a/doc/configuration.md b/doc/configuration.md index 29a7d50..0e48933 100644 --- a/doc/configuration.md +++ b/doc/configuration.md @@ -59,7 +59,7 @@ The administrators of the chat a person is in are admins there with no setting: | `future_days` | `30` | How many days ahead the events calendar looks | | `price_percent` | `5` | Movers threshold, percent: the moves `/movers`, `/session` and their jobs list | | `shorts_percent` | `15` | Short interest alert threshold, percent | -| `min_analysts` | `7` | `/buy`, `/sell`, `/pe` and `/peg` list only stocks covered by at least this many analysts; with fewer, a consensus or an estimate says little (thinly covered small caps are nearly all a perfect buy). Their charts say so in the subtitle | +| `min_analysts` | `7` | `/buy`, `/sell`, and `/ratio` (the forward P/E and the PEG) list only stocks covered by at least this many analysts; with fewer, a consensus or an estimate says little (thinly covered small caps are nearly all a perfect buy). Their charts say so in the subtitle | | `demote_volatile` | `True` | Volatile (small cap, high beta) stocks need a bigger move to alert | | `demote_leveraged` | `True` | Leveraged (2x, 3x) funds need a bigger move to alert | | `volatility_multiplier` | `2` | How much bigger a move volatile stocks need | diff --git a/doc/discord.md b/doc/discord.md index aaf5fec..a8c14d0 100644 --- a/doc/discord.md +++ b/doc/discord.md @@ -93,7 +93,7 @@ discordPublicKey=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcde ## Slash commands finbot registers a slash command for each line of `/help`, as it starts, and again only when the list has changed: -`/price`, `/dividend` and `/earnings`, `/buy` and `/sell`, `/pe`, `/stock`, `/jobs`, +`/price`, `/dividend` and `/earnings`, `/buy` and `/sell`, `/ratio`, `/stock`, `/jobs`, and the rest. Each takes its arguments, if any, as one text option, as a message would: `/movers` with `5 7d` is the message `.movers 5 7d`. They work in the server's channels and in a direct message with the bot. diff --git a/doc/slack.md b/doc/slack.md index c0ced7d..66dbe4e 100644 --- a/doc/slack.md +++ b/doc/slack.md @@ -107,7 +107,7 @@ Slack shows what changes, and asks to reinstall the app when its scopes do. ### Slash commands Each command is a slash command, with its usage as Slack's hint: `/price`, `/dividend` and `/earnings`, `/buy` and -`/sell`, `/pe`, `/stock`, `/jobs`, and the rest. +`/sell`, `/ratio`, `/stock`, `/jobs`, and the rest. Slack makes slash commands only from the app's settings, so they come in the manifest: when finbot's commands change, print it again and paste it into _App Manifest_. diff --git a/doc/telegram.md b/doc/telegram.md index 7b43c3f..c72e795 100644 --- a/doc/telegram.md +++ b/doc/telegram.md @@ -28,7 +28,7 @@ for several chats, then shows without an upload. A period pressed again within 3 shows that chart again with the report it came with: nothing is fetched or drawn either. As finbot starts, it sets the bot's command menu, which Telegram shows when `/` is typed: each command with its usage -(`/price`, `/pe`, `/stock` and the rest). `/profile` and `/job`, the second names of `/stock` and `/jobs`, are +(`/price`, `/ratio`, `/stock` and the rest). `/profile` and `/job`, the second names of `/stock` and `/jobs`, are left out of the menu (as on Slack and Discord), and work when typed, as every name of a command does. In a group, `/price@finbot_bot` is the same as `/price`. diff --git a/lib/reports/__init__.py b/lib/reports/__init__.py index 6d9c17f..59c2d48 100644 --- a/lib/reports/__init__.py +++ b/lib/reports/__init__.py @@ -4,7 +4,7 @@ posts nothing), and where a scheduled run takes arguments, job(argv), which reads them, and ARGUMENTS, what .jobs add lists (one without takes none: jobs.job_args); the scheduler runs them in the bot's process, and `.jobs run` runs one now. And the chat-only reports: about (.help, .jobs), history (.price, also called .compare and .history), holdings -(.holdings, .who, .treemap, .watchlist), pick (the pickers' buttons), profile (.SYMBOL) and rankings (.marketcap, .pe +(.holdings, .who, .treemap, .watchlist), pick (the pickers' buttons), profile (.SYMBOL) and rankings (.marketcap, .ratio and the rest).""" diff --git a/lib/reports/about.py b/lib/reports/about.py index 7472867..50eacf4 100644 --- a/lib/reports/about.py +++ b/lib/reports/about.py @@ -431,7 +431,7 @@ def prepare_reports(service): f"{jobs.takes(known[name])}".strip() for name in jobs.chat_actions()] + ['', service.bold('For example:'), '.jobs add movers weekly', '.jobs add movers close weekdays at 4:30pm New York', '.jobs add performance first day of each month at 9am', - '.jobs add trades every 20 mins weekdays from 10am to 4pm', '.jobs add peg top weekly', + '.jobs add trades every 20 mins weekdays from 10am to 4pm', '.jobs add ratio peg bottom weekly', '.jobs add price AAPL MSFT 1y monthly', '.jobs add reminder every 31 mar at 8:30am "πŸ₯³ Happy EOFY"', '', f"Times are by {when.place(formats.zone().key)}'s clock unless one names a zone (UTC, New York, EST);" " performance and movers cover the days between runs"] diff --git a/lib/reports/rankings.py b/lib/reports/rankings.py index aa6a8fb..74a15f1 100644 --- a/lib/reports/rankings.py +++ b/lib/reports/rankings.py @@ -1,4 +1,4 @@ -"""Ranked lists of the stocks followed (.marketcap, .pe and .peg, .beta, .buy and .sell), and one stock's +"""Ranked lists of the stocks followed (.marketcap, .ratio, .beta, .buy and .sell), and one stock's market cap or ratio.""" from lib import log from lib import answer diff --git a/lib/worker.py b/lib/worker.py index 93d9397..1275d38 100755 --- a/lib/worker.py +++ b/lib/worker.py @@ -503,18 +503,15 @@ def on_marketcap(request, args): else: request.reply(rankings.marketcap_list(request.service, request.dest.key, bottom=args.keyword == 'bottom')) -def on_ratio(kind): - """The handler of .pe (the forward P/E, or the current one given the option) or .peg (kind: forward pe or peg): the - top (by default) or bottom 15 by the ratio and its chart, the negative ones (where there can be any), or one - ticker's ratio.""" - def handler(request, args): - """.pe [SYMBOL|top|bottom|negative] [trailing|current] or .peg [SYMBOL|top|bottom|negative].""" - if args.words: - request.reply([rankings.ratio_line(request.service, ticker(args), kind)]) - else: - now = 'trailing pe' if args.option == 'trailing' else kind - request.reply(*rankings.ratio_list(request.service, request.dest.key, now, args.keyword or 'top')) - return handler +def on_ratio(request, args): + """.ratio [SYMBOL] [forward|current|peg] [top|bottom|negative|reverse]: the top (by default) or bottom (reverse) 15 + stocks by a ratio and its chart, the negative ones (where there can be any), or one ticker's ratio; the forward + P/E by default, the trailing one given current, the PEG given peg. .pe and .peg are its names, .peg's the PEG.""" + kind = args.option or ('peg' if args.word == 'peg' else 'forward pe') + if args.words: + request.reply([rankings.ratio_line(request.service, ticker(args), kind)]) + else: + request.reply(*rankings.ratio_list(request.service, request.dest.key, kind, args.keyword or 'top')) def on_beta(request, args): """.beta: the volatile small caps among the stocks followed.""" @@ -698,6 +695,8 @@ def fits(self, args): TOP = {'top': 'top', 'bottom': 'bottom'} # the ends of a ranked list NEGATIVE = TOP | dict.fromkeys(('neg', 'negative', 'negatives'), 'negative') # the ratios that can be negative +ENDS = NEGATIVE | dict.fromkeys(('reverse', 'reversed'), 'bottom') # .ratio's: the lowest first, or the highest +RATIOS = {'forward': 'forward pe', 'current': 'trailing pe', 'trailing': 'trailing pe', 'peg': 'peg'} # .ratio's DELETE = ('del', 'delete', 'rem', 'remove', 'rm') # how deleting is typed, to the watchlist and to .jobs CHANGE = {'add': 'add'} | dict.fromkeys(DELETE, 'delete') # of the watchlist BY = {word: name for name, words in holdings.NOUNS.items() for word in words} # .treemap's: sector, sectors, ... @@ -736,12 +735,9 @@ def fits(self, args): ('.movers [percent|top|bottom|session|premarket] [period]',), PRICE, ('.movers',), percent=True, period=True, keywords=TOP | {name: name for name in SESSIONS}, words=(0, 1)), # a word: session's or premarket's SYMBOL Command(BUY_SELL, on_rating, ('.buysell',), RISK, ('.buy & sell',)), # .buy & sell, .buy sell: both - Command(('pe', 'forward pe', 'forwardpe', 'fpe'), on_ratio('forward pe'), - ('.pe [SYMBOL|top|bottom|negative] [trailing]',), RISK, ('.pe',), keywords=NEGATIVE, - options={'trailing': 'trailing', 'current': 'trailing'}, words=(0, 1), - takes='ticker'), # the forward P/E: it was .forward pe - Command(('peg',), on_ratio('peg'), ('.peg [SYMBOL|top|bottom|negative]',), RISK, ('.peg',), keywords=NEGATIVE, - words=(0, 1), takes='ticker'), + Command(('ratio', 'pe', 'forward pe', 'forwardpe', 'fpe', 'peg'), on_ratio, + ('.ratio [SYMBOL] [forward|current|peg] [top|bottom|negative|reverse]',), RISK, ('.ratio',), keywords=ENDS, + options=RATIOS, words=(0, 1), takes='ticker'), # was .pe (the forward P/E) and .peg, still their names Command(('shorts', 'short'), on_shorts, ('.shorts [percent|SYMBOL]',), RISK, ('.shorts',), percent=True, words=(0, 1), takes='ticker'), Command(('trades', 'trade'), on_trades, ('.trades [period] [portfolio]',), TRADES, ('.trades',), period=True, @@ -860,7 +856,7 @@ def job_commands(): """The commands of .help a job may run, by each name a job may call one by: a CommandJob each. Every one but those that change something (job false) and those a scheduled report runs already (REPORTED): first each by the name its first line in .help that has one gives it (.stock's, not .SYMBOL's), with what it takes; then its other names and - its typos, each standing for the first, but buy and sell's (.compare stands for price).""" + its typos, each standing for the first, but buy and sell's and peg's (.compare stands for price).""" found = {} for command in COMMANDS: if not (command.job and command.usage) or command.handler in REPORTED: @@ -872,8 +868,8 @@ def job_commands(): first = found[name] = CommandJob(command, name, ' '.join(words[len(name.split()):])) break for name in command.names: - found.setdefault(name, dataclasses.replace(first, name=name if command.handler is on_rating else first.name, - listed=False)) + found.setdefault(name, dataclasses.replace(first, listed=False, name=name if command.handler is on_rating or + name == 'peg' else first.name)) # buy, sell and peg: the name says which return found PERIOD_WORDS = ('1d', '7d', '1m', '3m', '6m', 'ytd', '1y', '5y', '10y') # suggested where a command takes a period diff --git a/tests/test_discord.py b/tests/test_discord.py index 242e0f4..b4eb312 100644 --- a/tests/test_discord.py +++ b/tests/test_discord.py @@ -299,7 +299,7 @@ def test_each_help_line_is_a_slash_command_that_reads_as_its_command(self): commands = worker.slash_commands() names = [name for name, _, _, _ in commands] self.assertEqual(len(names), len(set(names))) - for expected in ('help', 'earnings', 'buysell', 'pe', 'stock', + for expected in ('help', 'earnings', 'buysell', 'ratio', 'stock', 'jobs', 'price', 'movers'): self.assertIn(expected, names) gone = {'profile', 'job', 'session', 'premarket', 'pie', 'plan', 'buy', 'sell', 'dividend'} @@ -360,7 +360,7 @@ def test_help_on_discord_lists_the_slash_commands(self): self.patch(transports.Transport, 'text', lambda dest, lines, buttons=None: sent.append(lines) or []) message = inbound.Message('discord', '77', '<@5>', '/help', '@99', '', '42/tok') worker.process_request(message) - self.assertIn('/pe [SYMBOL|top|bottom|negative] [trailing]', sent[0]) + self.assertIn('/ratio [SYMBOL] [forward|current|peg] [top|bottom|negative|reverse]', sent[0]) self.assertIn('/stock SYMBOL', sent[0]) self.assertIn('.price, or <@99> price', sent[0]) self.env(discordOutgoingWebhook='https://example.com/discord') diff --git a/tests/test_edges.py b/tests/test_edges.py index 13f2da1..4ef3ad6 100644 --- a/tests/test_edges.py +++ b/tests/test_edges.py @@ -243,7 +243,7 @@ def test_a_command_is_its_whole_name_in_any_case(self): """A command is called by its whole name, in any case and from any of its prefixes; a word that only starts with one is a ticker (.planet used to save the plan "et"), and the bot's name among the arguments is left out.""" for message, name in (('.MOVERS 7d', 'movers'), ('! movers', 'movers'), ('/movers@bot 7d', 'movers'), - ('@bot movers', 'movers'), ('.forward PE top', 'pe'), ('.PE', 'pe'), ('.buy/sell', 'buysell'), + ('@bot movers', 'movers'), ('.forward PE top', 'ratio'), ('.PE', 'ratio'), ('.buy/sell', 'buysell'), ('.movers @bot', 'movers'), ('.PRICE T1', 'price'), ('/price@bot T1', 'price'), ('.compare T1', 'price')): with self.subTest(message=message): diff --git a/tests/test_inbound.py b/tests/test_inbound.py index 6779a48..0ea840b 100644 --- a/tests/test_inbound.py +++ b/tests/test_inbound.py @@ -689,7 +689,7 @@ def test_every_message_and_the_slash_commands(self): commands = manifest['features']['slash_commands'] self.assertEqual([c['command'] for c in commands], ['/' + command.name for command in worker.slash_commands()]) names = {c['command'] for c in commands} - self.assertTrue({'/stock', '/jobs', '/pe'} <= names and not {'/profile', '/job'} & names) + self.assertTrue({'/stock', '/jobs', '/ratio'} <= names and not {'/profile', '/job'} & names) for command in commands: with self.subTest(command=command['command']): self.assertRegex(command['command'], r'^/[a-z0-9_]{1,31}$') diff --git a/tests/test_reports.py b/tests/test_reports.py index ae7ff2e..3291943 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -119,8 +119,8 @@ def test_the_keyboard_is_alphabetical_and_every_button_is_its_command(self): command it belongs to in the table, not taken for a ticker by .SYMBOL.""" buttons = [button['text'] for row in telegram_common.keyboard(worker.KEYBOARD)['keyboard'] for button in row] self.assertEqual(buttons, sorted(buttons)) - self.assertEqual(buttons, ['.buy & sell', '.earnings', '.movers', '.pe', '.peg', - '.performance', '.price', '.schedule', '.shorts', '.trades', '.treemap', '.watchlist']) + self.assertEqual(buttons, ['.buy & sell', '.earnings', '.movers', '.performance', + '.price', '.ratio', '.schedule', '.shorts', '.trades', '.treemap', '.watchlist']) for command in worker.COMMANDS: for text in command.buttons: with self.subTest(text=text): @@ -312,9 +312,10 @@ def listed(mode='top', kind='forward pe'): none() self.assertEqual(nothing.exception.lines, [said]) - def test_pe_takes_trailing_or_current_as_an_option_beside_its_other_words(self): - """.pe [top|bottom|negative] trailing (or current) lists the trailing P/E, of earnings reported, which needs no - analysts (a forward P/E does), with the option before or after the keyword; .peg takes it as a ticker.""" + def test_ratio_takes_the_ratio_then_its_end_as_optional_words(self): + """.ratio lists the forward P/E, lowest first; current (trailing) or peg picks another ratio, and top, bottom, + reverse (bottom) or negative the end, each optional and in either order; .pe and .peg are its names, .peg's + the PEG. A trailing P/E, of earnings reported, needs no analysts; a forward P/E and a PEG do.""" analysts = {'T1': 7, 'T2': 2, 'T3': 12} self.patch(watchlist, 'with_holdings', lambda chat: list(analysts)) trailing = {'T1': 30, 'T2': 10, 'T3': -4} @@ -328,14 +329,26 @@ def listed(text): charted.clear() self.command(text) return charted[0].title, [label for label, _ in charted[0].rows] - forward = listed('.pe') - self.assertEqual(forward[1], ['T1', 'T3']) # T2: too few analysts - for text in ('.pe trailing', '.pe current', '.pe trailing top', '.pe top current'): + for text in ('.ratio', '.pe', '.ratio forward', '.ratio top', '.ratio forward top', '.forward pe'): + with self.subTest(text=text): + title, tickers = listed(text) + self.assertEqual((tickers, 'forward pe ratio' in title), (['T1', 'T3'], True)) # T2: too few analysts + for text in ('.ratio current', '.ratio trailing', '.pe current', '.ratio top current', '.ratio current top'): with self.subTest(text=text): title, tickers = listed(text) self.assertEqual((tickers, 'trailing pe ratio' in title), (['T2', 'T1'], True)) # T3 is negative - self.assertEqual(listed('.pe bottom trailing')[1], ['T1', 'T2']) - self.assertEqual(listed('.pe negative current')[1], ['T3']) + for text, kind, tickers in (('.ratio current bottom', 'trailing pe', ['T1', 'T2']), + ('.ratio reverse current', 'trailing pe', ['T1', 'T2']), ('.pe bottom', 'forward pe', ['T3', 'T1']), + ('.ratio negative current', 'trailing pe', ['T3']), ('.ratio peg', 'peg', ['T1', 'T3']), + ('.peg', 'peg', ['T1', 'T3']), ('.peg reverse', 'peg', ['T3', 'T1']), + ('.ratio peg negative', 'peg', [])): + with self.subTest(text=text): + if tickers: + title, found = listed(text) + self.assertEqual((found, f'{kind} ratio' in title), (tickers, True)) + else: + charted.clear() + self.assertEqual(self.command(text)[0][0], 'text') # nothing negative: nothing to show self.assertEqual(rankings.ratio_list(TELEGRAM, 'telegram:55', 'trailing pe')[1].subtitle, formats.date_short(formats.today(), True)) # no analysts in a trailing chart's subtitle diff --git a/tests/test_schedule.py b/tests/test_schedule.py index 63213c4..48fc7d2 100644 --- a/tests/test_schedule.py +++ b/tests/test_schedule.py @@ -30,7 +30,7 @@ def test_a_job_runs_only_a_report_or_a_command_of_finbots_own(self): 'session', 'earnings', 'trades'} & set(commands)) self.assertEqual(jobs.chat_actions(), ['earnings', 'message', 'milestone', 'movers', 'performance', 'podcasts', 'rating', 'shorts', 'trades', 'holdings', 'treemap', 'who', 'marketcap', 'beta', 'price', - 'buysell', 'pe', 'peg', 'stock']) + 'buysell', 'ratio', 'stock']) def test_a_command_of_help_as_a_job(self): """A job may run a command of .help by its name, the words it takes before the schedule; another of its names @@ -49,8 +49,8 @@ def test_a_command_of_help_as_a_job(self): for text in ('beta x weekly', 'who weekly', 'stock weekly'): with self.subTest(text=text), self.assertRaises(jobs.ScheduleError): jobs.parse_add(text, 'UTC') - for action, args, made, name in (('hospital', ['AAPL'], 'compare', 'price AAPL'), ('fpe', [], 'pe', - 'pe'), ('buy&sell', [], 'buy&sell', 'buy & sell'), ('sell', [], 'sell', 'sell'), + for action, args, made, name in (('hospital', ['AAPL'], 'compare', 'price AAPL'), ('fpe', [], 'ratio', + 'ratio'), ('buy&sell', [], 'buy&sell', 'buy & sell'), ('sell', [], 'sell', 'sell'), ('price', ['AAPL'], 'compare', 'price AAPL')): with self.subTest(action=action): job = jobs.make_job(action, args, weekly, 'telegram:1') diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 33dc740..2b340e7 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -660,7 +660,8 @@ def test_add_alone_lists_the_reports_a_job_can_run(self): self.assertEqual(lines[0], 'What a job can run:') for line in ('reminder "TEXT"', 'performance [DAYS]', 'podcasts FEED_URL', 'movers midsession|interday|premarket|close|DAYS', 'trades', '.jobs add movers weekly', 'beta', - 'peg [SYMBOL|top|bottom|negative]', 'price [SYMBOL ... up to 8] [period]', '.jobs add peg top weekly'): + 'ratio [SYMBOL] [forward|current|peg] [top|bottom|negative|reverse]', + 'price [SYMBOL ... up to 8] [period]', '.jobs add ratio peg bottom weekly'): self.assertIn(line, lines) self.assertFalse(any(name in line for line in lines for name in jobs.SYSTEM_ACTIONS)) @@ -936,7 +937,7 @@ def test_add_by_buttons_back_and_cancel(self): self.assertEqual(lines[:3], ['Add a job: which report?', '', 'Jobs']) # the list stays in view self.assertEqual(rows, [['trades', 'movers midsession', 'movers premarket'], ['movers close', 'movers', 'performance'], ['earnings', 'shorts', 'rating'], ['milestone', 'reminder', 'holdings'], - ['treemap', 'who', 'marketcap'], ['beta', 'price', 'buysell'], ['pe', 'peg', 'stock'], ['Cancel']]) + ['treemap', 'who', 'marketcap'], ['beta', 'price', 'buysell'], ['ratio', 'stock'], ['Cancel']]) self.assertEqual((data['movers'], data['Cancel']), (f'm|add|5|{self.crc()}', f'm|top|0|{self.crc()}')) lines, rows, data = self.press(data['movers']) self.assertEqual((lines[:3], rows), (['Add movers: how often?', '', '† indicates default'], @@ -1054,10 +1055,10 @@ def test_the_commands_of_help_by_buttons(self): one whose usage brackets its stock or portfolio (a picker's, run bare as typed: the first one's answer); one that needs a stock says how to type it. On Discord the list comes on two pages.""" lines, rows, data = self.press(f'm|add|{self.crc()}') - for pressed in ('peg', 'weekly', 'Fri', '09†', '09:00†'): + for pressed in ('ratio', 'weekly', 'Fri', '09†', '09:00†'): lines, rows, data = self.press(data[pressed]) - self.assertEqual(newest(lines), '3. * peg Β· weekly, Fridays at 9:00am') - self.assertIn('peg', [job.action for job in self.s.chat_jobs(CHAT)]) + self.assertEqual(newest(lines), '3. * ratio Β· weekly, Fridays at 9:00am') + self.assertIn('ratio', [job.action for job in self.s.chat_jobs(CHAT)]) lines, rows, data = self.press(f'm|add|{self.crc()}') for pressed in ('treemap', 'weekly', 'Fri', '09†', '09:00†'): lines, rows, data = self.press(data[pressed]) @@ -1073,7 +1074,7 @@ def test_the_commands_of_help_by_buttons(self): self.assertEqual(([len(row) for row in first], [button.label for button in first[-1]]), ([4, 4, 4, 4, 2], ['β–Ά', 'Cancel'])) self.assertEqual([button.label for row in second for button in row], ['price', - 'buysell', 'pe', 'peg', 'stock', 'β—€', 'Cancel']) + 'buysell', 'ratio', 'stock', 'β—€', 'Cancel']) def test_a_step_that_is_none_adds_nothing(self): """Data that is no step of Add (forged, or an older finbot's: the weekdays frequency, the next 14 days, every... diff --git a/tests/test_telegram_api.py b/tests/test_telegram_api.py index 36a2ec7..0917d19 100644 --- a/tests/test_telegram_api.py +++ b/tests/test_telegram_api.py @@ -146,7 +146,8 @@ def call(method, **kwargs): self.assertEqual(calls, ['getMyCommands', 'setMyCommands', 'getMyCommands']) self.assertIn(f'telegram: {len(menu)} commands in the menu', log.getvalue()) names = [entry['command'] for entry in menu] - self.assertIn({'command': 'pe', 'description': 'pe [SYMBOL|top|bottom|negative] [trailing]'}, menu) + self.assertIn({'command': 'ratio', + 'description': 'ratio [SYMBOL] [forward|current|peg] [top|bottom|negative|reverse]'}, menu) self.assertEqual([name in names for name in ('stock', 'jobs', 'profile', 'job')], [True, True, False, False]) self.assertTrue(all(re.fullmatch(r'[a-z0-9_]{1,32}', name) for name in names)) self.patch(telegram_common, 'call', mock.Mock(side_effect=RuntimeError('Telegram getMyCommands: Unauthorized'))) diff --git a/util/record_outputs.py b/util/record_outputs.py index 583acd3..36203e4 100755 --- a/util/record_outputs.py +++ b/util/record_outputs.py @@ -17,7 +17,8 @@ REPO = Path(__file__).resolve().parent.parent COMMANDS = ['.help', '/start', '.hello', '.thanks', '.who T1', '.holdings', '.holdings Rob', '.marketcap', '.marketcap bottom', '.marketcap T1', '.pe', '.pe bottom', '.pe T1', - '.pe negative', '.pe trailing', '.pe bottom current', '.forward pe T1', '.peg', '.peg negative', '.peg T1', '.beta', + '.pe negative', '.ratio current', '.ratio peg reverse', '.ratio negative current', '.forward pe T1', '.peg', + '.peg negative', '.peg T1', '.beta', '.buy', '.sell', '.buy & sell', '.earnings', '.earnings T1', '.dividend', '.dividend T1', '.trades', '.trades 30d', '.trades 7d Nope', '.movers', '.movers 7d', '.movers 3', '.movers 3 7d', '.movers top', '.movers top 7d', '.price', '.price top', '.price T1', From eea3e1c8f9153993706ad1cec8dcd6f679ca6ffd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 09:43:44 +0000 Subject: [PATCH 03/25] doc/classes.md: every class, by what it is for Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- CLAUDE.md | 2 +- doc/classes.md | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 doc/classes.md diff --git a/CLAUDE.md b/CLAUDE.md index 4faf17a..68386a6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -262,5 +262,5 @@ An open PR takes in every commit pushed to its branch, so a PR opened for one sm - README.md is short: what finbot is, a quick start, and links. The rest is in `doc/`, a file per topic: `commands.md` (the commands and reports, 16 KB: grep for the section instead of reading it whole), `configuration.md` (the keys and settings), `scheduling.md`, `caching.md`, `install.md` (a Debian host), `docker.md`, and a guide per network, - `telegram.md`, `slack.md` and `discord.md`: a change to a network's setup or keys goes in its guide. `doc/yahoo/` and + `telegram.md`, `slack.md` and `discord.md`: a change to a network's setup or keys goes in its guide. `classes.md` lists every class by what it is for (keep it current when one is added, renamed or removed). `doc/yahoo/` and `doc/sharesight/` hold large API samples that the tests replay: inspect them with python, don't read them. diff --git a/doc/classes.md b/doc/classes.md new file mode 100644 index 0000000..e7a6af4 --- /dev/null +++ b/doc/classes.md @@ -0,0 +1,54 @@ +# Classes + +Every class in `bot.py` and `lib/`, by what it is for. A class named in two places (`Telegram`, `Slack`, `Discord`) is +a network's update reader in its `*_common.py` and a markup dialect in `lib/dialects.py`. + +## Networks: updates in, messages out +| Network | Reader of its updates | Bot that posts | Webhook (push) | Pull client | +|---|---|---|---|---| +| Telegram | `Telegram` | `TelegramBot` | `TelegramWebhook` | `TelegramPoll` (long polling) | +| Slack | `Slack` | `SlackBot` | `SlackWebhook` | `SlackSocket` (Socket Mode) | +| Discord | `Discord` | `DiscordBot` | `DiscordInteractions` | `DiscordGateway` | + +- **Bases:** `inbound.Network` (a reader), `inbound.Webhook`, `transports.Transport` (a bot), `pull.Listener` and + `pull.SocketListener` (a pull client; `pull.WebSocket` is its connection, `pull.Closed` raised when the server closes it). +- **What a reader hands on** (`lib/inbound.py`): `Message`, `Press` (a button pressed), `Suggest` (a slash command's + arguments as typed) and `Response` (what the network gets back at once); `Recent` remembers what was taken lately, to + drop repeats. +- **`bot.py`:** `WholeLines` writes each log line whole, whichever threads log at once; `Sent` is a response body that + starts some work once the server has sent it. + +## Markup and delivery +- `Dialect` (a `str` of the network's name) with `Telegram`, `Slack` and `Discord`: its markup, link syntax and limits. +- `delivery.Report` (lines, a chart spec, buttons) and `delivery.Preloader` (charts drawn ahead of presses). + +## Commands and buttons +- `worker.Command` (a row of the command table), `CommandJob` (a command run as a job), `Args` (what a command was + given), `Request` (a command being answered), `Slash` (a slash command to register) and `Usage` (raised for arguments a + command cannot take). +- `buttons.Button`; `ChartPress`, `MenuPress` and `PickPress` (a press, read from its data); `Act` (what a picker's + press asks for). + +## Scheduling +- `jobs.Scheduler` (runs the jobs) and `Job`; `catalog.Item` (a default job); `when.Schedule` (when a job runs). + +## Reports and charts +- `render.Chart` (a chart spec, drawn later); `render._Canvas` and `_Axes` (what a chart is drawn on). +- Data a report works from: `movers.Moves`, `trades.Trade`, `yahoo.Line` (a price line) and `markets.Market`. + +## Data sources and configuration +- `sharesight.Portfolio`. +- `config.Env` (finbot.env, fixed at start), `Settings` (settings.conf, new on each reload) and the private `_Reader` + that reads their typed fields. + +## Errors +- **Replies, not failures:** `answer.Answer` (lines to reply with in place of the report) and `answer.Nothing` (nothing + to show: said only to a command or a button, never in a job's run). Both are also raised by: + - `sharesight.NotSetUp` and `PortfolioNotFound`; + - `yahoo.NoData`. +- **Failures:** + - `net.RateLimited` (a 429 and the wait it asks for); + - `transports.MaybeSent` (a chart's post timed out: it may have arrived); + - `sharesight.Refused` (credentials refused: the token is renewed); + - `when.ScheduleError` (a schedule or job that cannot be); + - `config.ConfigError` (problems in finbot.env or settings.conf, each naming its file and line). From e610a5c8aae661063a71a73e101ff8baebf06db2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 10:15:12 +0000 Subject: [PATCH 04/25] doc/slack.md: say why both manifests must not be installed Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/slack.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/slack.md b/doc/slack.md index 66dbe4e..94d4b6b 100644 --- a/doc/slack.md +++ b/doc/slack.md @@ -27,8 +27,8 @@ switch, change both (paste the manifest for the other mode, which sets the app's 1. Print the app's manifest, which sets up the whole app at once (the slash commands, the scopes, the events, the buttons, direct messages and Socket Mode): `util/slack_manifest.py`. The bot then hears the messages that mention it (`@finbot price`), its slash commands and direct messages. With `--every-message`, it hears every message in its - channels instead of the mentions, so that `.price` works there without mentioning it; not both, or it would answer a - mention twice. + channels instead, so that `.price` works there without mentioning it. Do not install both manifests, or a mention + would reach it twice, as a message and as a mention, and it would answer twice. 2. At https://api.slack.com/apps choose _Create New App > From an app manifest_, pick your workspace, paste the manifest into the JSON tab, and _Create_. 3. Under _Basic Information > App-Level Tokens_, choose _Generate Token and Scopes_: name it (finbot, say), add the From 0a9cf172c8c05b48c2a2b2ee0c15d5216c2abea0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 10:21:33 +0000 Subject: [PATCH 05/25] Slack manifest leaves out /help, which Slack keeps for itself Slack refused the manifest ("The slash command has an invalid name") at /help, one of its own commands. The manifest names every slash command of .help but that one; .help and @finbot help still answer. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/slack.md | 2 ++ tests/test_inbound.py | 3 ++- util/slack_manifest.py | 6 ++++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/doc/slack.md b/doc/slack.md index 94d4b6b..65a4bf5 100644 --- a/doc/slack.md +++ b/doc/slack.md @@ -108,6 +108,8 @@ Slack shows what changes, and asks to reinstall the app when its scopes do. Each command is a slash command, with its usage as Slack's hint: `/price`, `/dividend` and `/earnings`, `/buy` and `/sell`, `/ratio`, `/stock`, `/jobs`, and the rest. +Not `/help`, which Slack keeps for its own (a manifest that names it is refused as an invalid name): `.help` and +`@finbot help` work. Slack makes slash commands only from the app's settings, so they come in the manifest: when finbot's commands change, print it again and paste it into _App Manifest_. diff --git a/tests/test_inbound.py b/tests/test_inbound.py index 0ea840b..2bfdb93 100644 --- a/tests/test_inbound.py +++ b/tests/test_inbound.py @@ -687,7 +687,8 @@ def test_every_message_and_the_slash_commands(self): self.assertIn('channels:history', manifest['oauth_config']['scopes']['bot']) self.assertNotIn('app_mentions:read', manifest['oauth_config']['scopes']['bot']) commands = manifest['features']['slash_commands'] - self.assertEqual([c['command'] for c in commands], ['/' + command.name for command in worker.slash_commands()]) + self.assertEqual([c['command'] for c in commands], ['/' + command.name for command in worker.slash_commands() + if command.name != 'help']) # Slack keeps /help: a manifest that names it is refused names = {c['command'] for c in commands} self.assertTrue({'/stock', '/jobs', '/ratio'} <= names and not {'/profile', '/job'} & names) for command in commands: diff --git a/util/slack_manifest.py b/util/slack_manifest.py index 37cded9..5a30c95 100755 --- a/util/slack_manifest.py +++ b/util/slack_manifest.py @@ -24,6 +24,7 @@ SCOPES = ('chat:write', 'commands', 'files:read', 'files:write', 'reactions:write', 'users:read') # replies, slash commands, charts, ⏳, who the workspace's admins are +RESERVED = ('help',) # slash commands Slack keeps for itself: a manifest that names one is refused ("invalid name") EVENT_SCOPES = {'app_mention': 'app_mentions:read', 'message.channels': 'channels:history', 'message.groups': 'groups:history', 'message.im': 'im:history'} # what each event needs @@ -31,13 +32,14 @@ def manifest(name='finbot', url='', every_message=False): """The manifest, as a dict: for Socket Mode without url; for HTTP mode with it, the Request URL of the events, the buttons and each slash command. The bot hears the messages that mention it, or with every_message each message in - the channels it is in, and its direct messages either way.""" + the channels it is in, and its direct messages either way. Every slash command of .help but /help, which Slack's + own is (.help and @finbot help work).""" events = ['message.channels', 'message.groups'] if every_message else ['app_mention'] events.append('message.im') posted = {'url': url} if url else {} commands = [{'command': '/' + command.name, 'description': command.description, 'should_escape': False, **({'usage_hint': command.arguments} if command.arguments else {}), **posted} - for command in worker.slash_commands()] + for command in worker.slash_commands() if command.name not in RESERVED] return { 'display_information': {'name': name, 'description': 'Stock prices, charts and Sharesight reports', 'background_color': '#14181f'}, From d2e9d1baee20aa614c9d983031376c04ba4c1894 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 10:25:01 +0000 Subject: [PATCH 06/25] slack_manifest.py --only NAMES: a manifest with just those slash commands To find which slash command Slack refuses when it rejects a manifest. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/slack.md | 2 ++ tests/test_inbound.py | 2 ++ util/slack_manifest.py | 11 +++++++---- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/doc/slack.md b/doc/slack.md index 65a4bf5..1d9f783 100644 --- a/doc/slack.md +++ b/doc/slack.md @@ -110,6 +110,8 @@ Each command is a slash command, with its usage as Slack's hint: `/price`, `/div `/sell`, `/ratio`, `/stock`, `/jobs`, and the rest. Not `/help`, which Slack keeps for its own (a manifest that names it is refused as an invalid name): `.help` and `@finbot help` work. +If Slack refuses a manifest, `util/slack_manifest.py --only price,who` prints it with just those slash commands, to find +which one it objects to. Slack makes slash commands only from the app's settings, so they come in the manifest: when finbot's commands change, print it again and paste it into _App Manifest_. diff --git a/tests/test_inbound.py b/tests/test_inbound.py index 2bfdb93..b98fb9b 100644 --- a/tests/test_inbound.py +++ b/tests/test_inbound.py @@ -690,6 +690,8 @@ def test_every_message_and_the_slash_commands(self): self.assertEqual([c['command'] for c in commands], ['/' + command.name for command in worker.slash_commands() if command.name != 'help']) # Slack keeps /help: a manifest that names it is refused names = {c['command'] for c in commands} + only = slack_manifest.manifest(only={'price', 'who'})['features']['slash_commands'] + self.assertEqual([c['command'] for c in only], ['/who', '/price']) # to find which command Slack refuses self.assertTrue({'/stock', '/jobs', '/ratio'} <= names and not {'/profile', '/job'} & names) for command in commands: with self.subTest(command=command['command']): diff --git a/util/slack_manifest.py b/util/slack_manifest.py index 5a30c95..e76b242 100755 --- a/util/slack_manifest.py +++ b/util/slack_manifest.py @@ -7,6 +7,7 @@ util/slack_manifest.py --url https://example.com/slack HTTP mode: Slack posts to slackOutgoingWebhook, this URL util/slack_manifest.py --every-message the bot sees every message in its channels, not only the ones that mention it, so that .price works there + util/slack_manifest.py --only price,who just these slash commands: to find which one Slack refuses Slack makes slash commands only in an app's settings (there is no API for them without a configuration token), so this is the way they get there: run it again, and paste what it prints, when finbot's commands change.""" @@ -29,17 +30,17 @@ 'message.groups': 'groups:history', 'message.im': 'im:history'} # what each event needs -def manifest(name='finbot', url='', every_message=False): +def manifest(name='finbot', url='', every_message=False, only=()): """The manifest, as a dict: for Socket Mode without url; for HTTP mode with it, the Request URL of the events, the buttons and each slash command. The bot hears the messages that mention it, or with every_message each message in the channels it is in, and its direct messages either way. Every slash command of .help but /help, which Slack's - own is (.help and @finbot help work).""" + own is (.help and @finbot help work); only, those named.""" events = ['message.channels', 'message.groups'] if every_message else ['app_mention'] events.append('message.im') posted = {'url': url} if url else {} commands = [{'command': '/' + command.name, 'description': command.description, 'should_escape': False, **({'usage_hint': command.arguments} if command.arguments else {}), **posted} - for command in worker.slash_commands() if command.name not in RESERVED] + for command in worker.slash_commands() if command.name not in RESERVED and (not only or command.name in only)] return { 'display_information': {'name': name, 'description': 'Stock prices, charts and Sharesight reports', 'background_color': '#14181f'}, @@ -66,11 +67,13 @@ def main(argv=None): parser.add_argument('--url', default='', help='HTTP mode: slackOutgoingWebhook, which Slack posts to') parser.add_argument('--name', default='finbot', help="the app's and the bot's name (default finbot)") parser.add_argument('--every-message', action='store_true', help='the bot sees every message in its channels') + parser.add_argument('--only', default='', help='only these slash commands (names, comma separated)') args = parser.parse_args(argv) if args.url and not args.url.startswith('https://'): print('slack_manifest: --url must be https://: Slack posts to no other', file=sys.stderr) return 2 - print(json.dumps(manifest(args.name, args.url, args.every_message), indent=2, ensure_ascii=False)) + print(json.dumps(manifest(args.name, args.url, args.every_message, set(args.only.split(',')) - {''}), indent=2, + ensure_ascii=False)) return 0 From 466564968325f8149b98951b79af3e02d3b96c4e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 10:29:00 +0000 Subject: [PATCH 07/25] Slack manifest leaves out /who, which Slack keeps for itself; /help is back Slack refused the manifest at /who ("invalid name"), found with --only. /help was never the problem, so the manifest names it again; .who SYMBOL and @finbot who SYMBOL still answer. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/slack.md | 7 +++---- tests/test_inbound.py | 6 +++--- util/slack_manifest.py | 8 ++++---- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/doc/slack.md b/doc/slack.md index 1d9f783..c2a8512 100644 --- a/doc/slack.md +++ b/doc/slack.md @@ -108,10 +108,9 @@ Slack shows what changes, and asks to reinstall the app when its scopes do. Each command is a slash command, with its usage as Slack's hint: `/price`, `/dividend` and `/earnings`, `/buy` and `/sell`, `/ratio`, `/stock`, `/jobs`, and the rest. -Not `/help`, which Slack keeps for its own (a manifest that names it is refused as an invalid name): `.help` and -`@finbot help` work. -If Slack refuses a manifest, `util/slack_manifest.py --only price,who` prints it with just those slash commands, to find -which one it objects to. +Not `/who`, which Slack keeps for its own (a manifest that names it is refused as an invalid name): `.who SYMBOL` +and `@finbot who SYMBOL` work. If Slack refuses a manifest, `util/slack_manifest.py --only price,stock` prints it with +just those slash commands, to find which one it objects to. Slack makes slash commands only from the app's settings, so they come in the manifest: when finbot's commands change, print it again and paste it into _App Manifest_. diff --git a/tests/test_inbound.py b/tests/test_inbound.py index b98fb9b..13a8c9f 100644 --- a/tests/test_inbound.py +++ b/tests/test_inbound.py @@ -688,10 +688,10 @@ def test_every_message_and_the_slash_commands(self): self.assertNotIn('app_mentions:read', manifest['oauth_config']['scopes']['bot']) commands = manifest['features']['slash_commands'] self.assertEqual([c['command'] for c in commands], ['/' + command.name for command in worker.slash_commands() - if command.name != 'help']) # Slack keeps /help: a manifest that names it is refused + if command.name != 'who']) # Slack keeps /who: a manifest that names it is refused names = {c['command'] for c in commands} - only = slack_manifest.manifest(only={'price', 'who'})['features']['slash_commands'] - self.assertEqual([c['command'] for c in only], ['/who', '/price']) # to find which command Slack refuses + only = slack_manifest.manifest(only={'price', 'stock'})['features']['slash_commands'] + self.assertEqual([c['command'] for c in only], ['/price', '/stock']) # to find which command Slack refuses self.assertTrue({'/stock', '/jobs', '/ratio'} <= names and not {'/profile', '/job'} & names) for command in commands: with self.subTest(command=command['command']): diff --git a/util/slack_manifest.py b/util/slack_manifest.py index e76b242..475fb4b 100755 --- a/util/slack_manifest.py +++ b/util/slack_manifest.py @@ -7,7 +7,7 @@ util/slack_manifest.py --url https://example.com/slack HTTP mode: Slack posts to slackOutgoingWebhook, this URL util/slack_manifest.py --every-message the bot sees every message in its channels, not only the ones that mention it, so that .price works there - util/slack_manifest.py --only price,who just these slash commands: to find which one Slack refuses + util/slack_manifest.py --only price,stock just these slash commands: to find which one Slack refuses Slack makes slash commands only in an app's settings (there is no API for them without a configuration token), so this is the way they get there: run it again, and paste what it prints, when finbot's commands change.""" @@ -25,7 +25,7 @@ SCOPES = ('chat:write', 'commands', 'files:read', 'files:write', 'reactions:write', 'users:read') # replies, slash commands, charts, ⏳, who the workspace's admins are -RESERVED = ('help',) # slash commands Slack keeps for itself: a manifest that names one is refused ("invalid name") +RESERVED = ('who',) # slash commands Slack keeps for itself: a manifest that names one is refused ("invalid name") EVENT_SCOPES = {'app_mention': 'app_mentions:read', 'message.channels': 'channels:history', 'message.groups': 'groups:history', 'message.im': 'im:history'} # what each event needs @@ -33,8 +33,8 @@ def manifest(name='finbot', url='', every_message=False, only=()): """The manifest, as a dict: for Socket Mode without url; for HTTP mode with it, the Request URL of the events, the buttons and each slash command. The bot hears the messages that mention it, or with every_message each message in - the channels it is in, and its direct messages either way. Every slash command of .help but /help, which Slack's - own is (.help and @finbot help work); only, those named.""" + the channels it is in, and its direct messages either way. Every slash command of .help but /who, which Slack's + own is (.who SYMBOL and @finbot who SYMBOL work); only, those named.""" events = ['message.channels', 'message.groups'] if every_message else ['app_mention'] events.append('message.im') posted = {'url': url} if url else {} From a167756ec5df3cdac616804b1bf7c94a1f4198e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 10:45:15 +0000 Subject: [PATCH 08/25] doc/slack.md: the manifest's mode comes from --url, not from finbot.env The text implied util/slack_manifest.py reads slackOutgoingWebhook, which the tool never does (finbot.env is readable by root alone). It is the tool's --url that picks push mode; finbot.env picks finbot's own side. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/slack.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/doc/slack.md b/doc/slack.md index c2a8512..6ce7179 100644 --- a/doc/slack.md +++ b/doc/slack.md @@ -18,9 +18,13 @@ The bot does the same either way. What differs is how it hears from Slack: | Python module | `websocket-client` (Debian: `python3-websocket`) | `waitress` (Debian: `python3-waitress`) | | `finbot.env` keys | `slackBotToken`, `slackAppToken` | `slackBotToken`, `slackOutgoingWebhook`, `slackSigningSecret` | -`slackOutgoingWebhook` chooses the mode: set, push mode; empty, Socket Mode. The app's own _Socket Mode_ setting must -agree, as Slack sends to one or the other: to finbot's socket while it is on, to the Request URLs while it is off. To -switch, change both (paste the manifest for the other mode, which sets the app's side) and restart finbot. +finbot chooses its mode from `finbot.env`: with `slackOutgoingWebhook` set it is push mode, with it empty Socket +Mode. The Slack app has its own _Socket Mode_ setting, and the two must agree, as Slack sends to one or the other: to +finbot's socket while the app's is on, to the Request URLs while it is off. The app's side is set by the manifest, which +`util/slack_manifest.py` makes without reading `finbot.env`: run without `--url` it is Socket Mode, and with `--url +https://example.com/slack` (the same URL as `slackOutgoingWebhook`) push mode. To switch, do both: paste the manifest +for the other mode into the app, and change `finbot.env` to match (set `slackOutgoingWebhook` for push mode, empty it +for Socket Mode), then restart finbot. ### Socket Mode From 030a9a143e533e531c980f607979524e2d6bf572 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 10:54:53 +0000 Subject: [PATCH 09/25] Slack: say why a press uploads its chart, and word the not-ready-yet retry plainly Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- lib/slack_common.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/slack_common.py b/lib/slack_common.py index 27b7bd6..70d9dc6 100644 --- a/lib/slack_common.py +++ b/lib/slack_common.py @@ -231,6 +231,8 @@ def showing(self, data, post): raise log.warn("Slack will not show an earlier upload of the chart, uploading it anew:", net.redact(e)) self.forget(earlier) + if not config.settings.slack_preload_chat: + log.info("Slack uploads the chart for this press: slack_preload_chat is not set, none was sent ahead") file_id, _ = self.upload(data) return until_shown(lambda: post(file_id)), file_id, digest @@ -389,7 +391,7 @@ def until_shown(post, waits=(1, 2, 4)): except RuntimeError as e: if 'invalid_blocks' not in str(e): raise - log.info(f"Slack cannot show the chart yet ({e}), trying again in {wait} s") + log.info(f"Slack is not ready to show the chart just uploaded ({e}), trying again in {wait} s") time.sleep(wait) return post() From 5905a37cab156ada25977023da98c8d299a75520 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 14:51:54 +0000 Subject: [PATCH 10/25] A bar chart of one bar is skipped: the report goes as text Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/commands.md | 2 +- lib/charts.py | 2 +- lib/delivery.py | 4 ++-- lib/render.py | 8 ++++++++ lib/worker.py | 2 ++ tests/common.py | 2 +- tests/test_reports.py | 10 +++++----- tests/test_sending.py | 11 +++++++++++ 8 files changed, 31 insertions(+), 10 deletions(-) diff --git a/doc/commands.md b/doc/commands.md index 6b75ac6..84f3a8e 100644 --- a/doc/commands.md +++ b/doc/commands.md @@ -70,7 +70,7 @@ The bot answers commands in every chat it is in. By default, it fetches its upda * Running the other stock reports on demand ## Reports -Every chart's subtitle ends with the date it shows, or the dates it spans (`12 Oct - 19 Oct 2026`). +Every chart's subtitle ends with the date it shows, or the dates it spans (`12 Oct - 19 Oct 2026`). A bar chart of a single bar is not drawn: the report goes as its text, with its buttons. ### Stock lookup ![Screenshot showing stock info on Slack](../img/stockinfo.png?raw=true "Screenshot showing stock info on Slack") diff --git a/lib/charts.py b/lib/charts.py index 6348a3f..5a3e065 100644 --- a/lib/charts.py +++ b/lib/charts.py @@ -87,7 +87,7 @@ def prime(service, wanted): for label, make in wanted: try: (_, chart), built = timed(make) - if chart and chart.rows: + if chart and chart.drawable: image, drew = timed(render.draw, chart) preloader.add(image) primed(label, built, drew) diff --git a/lib/delivery.py b/lib/delivery.py index 40d80c2..6ffa469 100644 --- a/lib/delivery.py +++ b/lib/delivery.py @@ -101,8 +101,8 @@ def deliver(dest, report): lines following as text; refused again, the lines go as text alone, without its buttons. A chart that may have arrived after all (a timeout) is not sent again. Returns the network's answers, one per message.""" chart = report.chart - if chart is None or not (dest.takes_charts and config.settings.graph and chart.rows): - return dest.text(report.lines, report.buttons if chart is None else None) + if chart is None or not (dest.takes_charts and config.settings.graph and chart.drawable): + return dest.text(report.lines, report.buttons if chart is None or (chart.rows and not chart.drawable) else None) caption = '\n'.join(map(str, report.lines)) or chart.title image = render.draw(chart) answer = dest.chart(image, caption, report.buttons) diff --git a/lib/render.py b/lib/render.py index 95c4064..6db4a19 100644 --- a/lib/render.py +++ b/lib/render.py @@ -34,6 +34,14 @@ class Chart: threshold: float = None # shorts: the outrageous line change: float = None # line: the change shown, whose sign colours the line and its subtitle + @property + def drawable(self): + """Whether the chart is worth drawing: it has rows, and bars have more than one (a single bar says no more than + the report's line does).""" + if self.kind == 'bars': + return sum(value is not None for _, value in self.rows) > 1 + return bool(self.rows) + VALUE_FORMATS = {'percent': lambda v: formats.signed_percent(round(v)), 'percent2': lambda v: formats.signed_percent(v, 2), 'number': lambda v: f"{v:g}"} diff --git a/lib/worker.py b/lib/worker.py index 1275d38..144caf0 100755 --- a/lib/worker.py +++ b/lib/worker.py @@ -266,6 +266,8 @@ def chart_press(chat, press, pressed): if chat.reshow(press.message_id, markup): # that period as posted lately (Slack, Telegram): nothing built return lines, chart = charts.build(pressed.kind, args, chat.dialect, chat.key, period=pressed.period) + if chart and chart.rows and not chart.drawable: # a lone bar: the text alone + return chat.edit_lines(press.message_id, lines, markup) if not (chart and chart.rows): # too few prices in that period to draw a line raise answer.Answer(f"Too few prices to draw {buttons.PERIODS[pressed.period][0]}") chat.edit_chart(press.message_id, render.draw(chart), '\n'.join(lines), markup) diff --git a/tests/common.py b/tests/common.py index 44cbbc8..58262a5 100644 --- a/tests/common.py +++ b/tests/common.py @@ -258,7 +258,7 @@ def get(url, **kwargs): REAL_CREDENTIALS = yahoo.credentials -CHART = render.Chart('bars', 'Heading', rows=(('A', 1.0),)) # a chart for the sending tests, which draw it as b'png' +CHART = render.Chart('bars', 'Heading', rows=(('A', 1.0), ('B', 2.0))) # for the sending tests, drawn as b'png' REAL_TOKEN = sharesight._token diff --git a/tests/test_reports.py b/tests/test_reports.py index 3291943..f617195 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -322,8 +322,8 @@ def test_ratio_takes_the_ratio_then_its_end_as_optional_words(self): self.patch(yahoo, 'fetch', lambda ts: {t: market_entry(t) | {'price_to_earnings_forward': 20, 'price_to_earnings_trailing': trailing[t]} for t in ts}) self.patch(yahoo, 'fetch_detail', lambda t, *a, **k: {t: market_entry(t) | {'recommend_analysts': analysts[t]}}) - charted, real = [], render.draw - self.patch(render, 'draw', lambda chart: charted.append(chart) or real(chart)) + charted, real = [], delivery.deliver + self.patch(delivery, 'deliver', lambda dest, report: charted.append(report.chart) or real(dest, report)) def listed(text): """(the chart's title, the tickers it shows) of the list a command gives.""" charted.clear() @@ -1591,7 +1591,7 @@ def build(kind, args, service, chat, period=None, days=None): raise RuntimeError('boom') if period == 'y': raise answer.Nothing('No price movements meet threshold 5%') - return ['caption'], None if period == 'd' else render.Chart('bars', rows=(('a', 1.0),)) + return ['caption'], None if period == 'd' else render.Chart('bars', rows=(('a', 1.0), ('b', 2.0))) self.patch(charts, 'build', build) with contextlib.redirect_stderr(io.StringIO()) as err: charts.prime(TELEGRAM, charts.periods('l', ['5', '0'], TELEGRAM, 'telegram:55', 'x')) @@ -1625,7 +1625,7 @@ def draw(chart): return chart.title self.patch(render, 'draw', draw) self.patch(charts, 'build', lambda kind, args, service, chat, period=None, days=None: ('caption', - render.Chart('bars', period, rows=(('a', 1.0),)))) + render.Chart('bars', period, rows=(('a', 1.0), ('b', 2.0))))) charts.prime(TELEGRAM, charts.periods('c', ['T1'], TELEGRAM, 'telegram:55', 'x')) self.assertEqual(drawn, [('w', []), ('m', ['w']), ('y', ['w', 'm'])]) # each was handed over before the next self.assertEqual((made[0].service, made[0].images, made[0].finished), (TELEGRAM, ['w', 'm', 'y'], True)) @@ -1643,7 +1643,7 @@ def test_priming_logs_how_long_each_chart_took_to_build_and_draw(self): self.patch(charts, 'time', types.SimpleNamespace(monotonic=lambda: next(ticks))) self.patch(render, 'draw', lambda chart: chart.title) self.patch(charts, 'build', lambda kind, args, service, chat, period=None, days=None: ('caption', - render.Chart('bars', period, rows=(('a', 1.0),)))) + render.Chart('bars', period, rows=(('a', 1.0), ('b', 2.0))))) with contextlib.redirect_stderr(io.StringIO()) as log: charts.prime(TELEGRAM, charts.periods('c', ['T1'], TELEGRAM, 'telegram:55', 'x')) self.assertNotIn('priming', log.getvalue()) diff --git a/tests/test_sending.py b/tests/test_sending.py index 8ad35eb..285359e 100644 --- a/tests/test_sending.py +++ b/tests/test_sending.py @@ -538,6 +538,17 @@ def test_the_graph_setting_off_sends_no_chart_anywhere(self): self.assertEqual({kind for kind, *_ in self.sent}, {'text'}) self.assertEqual([url for url in posts if 'sendPhoto' in url or 'files.' in url], []) + def test_a_bar_chart_of_one_item_goes_as_text_with_its_buttons(self): + """A report whose bar chart has a single bar goes as its text alone; a gap is no bar.""" + drawn = [] + self.patch(render, 'draw', lambda chart: drawn.append(chart) or io.BytesIO(b'png')) + chat = delivery.reply_to(TELEGRAM, '55') + for rows, charts in (((('A', 1.0),), 0), ((('A', 1.0), ('', None)), 0), ((('A', 1.0), ('B', 2.0)), 1)): + with self.subTest(rows=rows): + drawn.clear() + delivery.deliver(chat, delivery.Report(['Heading', 'line'], render.Chart('bars', 'T', rows=rows), [])) + self.assertEqual(len(drawn), charts) + def test_a_chart_telegram_refuses_goes_out_as_text(self): """A chart Telegram refuses or that fails goes as text, but not after a timeout, when it may have arrived.""" self.patch(render, 'draw', lambda chart: io.BytesIO(b'png')) From fb1cac0cafe23f8c94cd056cf52619b93630bd90 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 00:02:47 +0000 Subject: [PATCH 11/25] jobs > Edit: Disable all button; /jobs top level has no page buttons Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/scheduling.md | 4 ++-- lib/reports/about.py | 18 +++++++++++++----- lib/worker.py | 12 +++++++++--- tests/test_scheduler.py | 13 +++++++------ 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/doc/scheduling.md b/doc/scheduling.md index 17e5d1a..3cebdb4 100644 --- a/doc/scheduling.md +++ b/doc/scheduling.md @@ -9,7 +9,7 @@ direct message with the bot, until the chat switches them. finbot knows a chat f channel on Slack, and on Discord a message from no server. ## The menu -`/jobs` lists the chat's jobs, numbered, a page at a time, the jobs of each report together under a heading with its name (Movers, Shorts, ...), the reminders last under theirs. Each line is struck through and marked ❌ when the job is off, with a `*` if the job has While a job is added, each step shows the list under its question. +`/jobs` lists the chat's jobs, numbered (the top level shows as many as fit a message, then how many more there are, with no page buttons: Edit and Run list every job, a page at a time), the jobs of each report together under a heading with its name (Movers, Shorts, ...), the reminders last under theirs. Each line is struck through and marked ❌ when the job is off, with a `*` if the job has While a job is added, each step shows the list under its question. never run, its name and its schedule in words. Under the list, in italics, is how Add is typed: ``` 1. trades Β· every 20 mins, Mon 9am – Sat 3pm Sydney @@ -22,7 +22,7 @@ never run, its name and its schedule in words. Under the list, in italics, is ho ``` Under it is one row of buttons, each there only where it has something to act on (**+** always; **Edit** and **Run** with any job, **Upcoming** with one that is on). Each screen is drawn in place of the last: * **+**: what the job runs, how often, then the rest down to the minute ([below](#add-step-by-step)). -* **Edit**: the list with a button per job, the default ones too; a press switches that job on or off at once. A job that is off is struck through in the list and has a red button (on Telegram, Slack and Discord alike); one that is on has a plain one. **OK** goes back to the list. **Delete disabled jobs**, there while any job is off, asks first ("Delete the 3 disabled jobs?"), then deletes them. A default job that is deleted comes back when the same job is added again. +* **Edit**: the list with a button per job, the default ones too; a press switches that job on or off at once. A job that is off is struck through in the list and has a red button (on Telegram, Slack and Discord alike); one that is on has a plain one. **OK** goes back to the list. **Disable all**, there while any job is on, switches them all off at once. **Delete disabled jobs**, there while any job is off, asks first ("Delete the 3 disabled jobs?"), then deletes them. A default job that is deleted comes back when the same job is added again. * **Run**: a button per job; a press runs it now, as its schedule would. What the job posts shows it ran; a message says so only when it failed, or was running already. The same button pressed again within a minute does nothing more, as with every button. diff --git a/lib/reports/about.py b/lib/reports/about.py index 50eacf4..09fb322 100644 --- a/lib/reports/about.py +++ b/lib/reports/about.py @@ -60,7 +60,9 @@ def jobs_screen(service, chat, screen='top', page=0, note=None, highlight=None): if highlight: entries = [(number, job, f'πŸ‘‰ {service.bold(line)}' if number == highlight else line) for number, job, line in entries] - parts = paged(entries, service) + parts = paged(entries, service, screen != 'top') + if screen == 'top': # no pages: what fits, and how many more Edit and Run list + parts = parts[:1] if highlight: page = next((i for i, part in enumerate(parts) if any(number == highlight for number, _, _ in part)), page) page = min(max(page, 0), len(parts) - 1) @@ -71,6 +73,8 @@ def jobs_screen(service, chat, screen='top', page=0, note=None, highlight=None): lines.append('No jobs here: + adds one') if shown: lines += grouped(service, shown) + if screen == 'top' and shown and len(shown) < len(own): + lines += ['', f'…and {len(own) - len(shown)} more: Edit and Run list every job'] if not all(scheduler.has_run(job) for _, job, _ in shown): lines += ['', NOT_RUN] if screen == 'top': @@ -108,10 +112,11 @@ def listed_with(service, chat, lines): more = [f'…and {len(own) - len(shown)} more'] if len(shown) < len(own) else [] return [*lines, '', service.bold('Jobs'), *grouped(service, shown)[1:], *more] -def paged(entries, service): +def paged(entries, service, with_buttons=True): """The jobs' entries (number, job, line) a page at a time: as many as fit a message of the network with room to - spare, and as many buttons as it takes in a message besides the others.""" - most, room = service.room(JOB_ROW, otherwise=MOST_JOBS), service.text_limit - PAGE_ROOM + spare, and (with_buttons: a button each) as many buttons as it takes in a message besides the others.""" + most = service.room(JOB_ROW, otherwise=MOST_JOBS) if with_buttons else len(entries) or 1 + room = service.text_limit - PAGE_ROOM parts, size = [[]], 0 for entry in entries: if parts[-1] and (len(parts[-1]) == most or size + len(entry[2]) + 1 > room): @@ -124,7 +129,8 @@ def paged(entries, service): def screen_buttons(screen, page, pages, shown, crc, own): """A list screen's buttons: top's, what can be done (+; Edit and Run with any job, Upcoming with one on), in one row; edit's and run's, a button for each job shown, ' ', JOB_ROW to a row, edit's red for a job that - is off, then OK and, while any job is off, Delete disabled jobs (edit), or Cancel (run); β—€ and β–Ά to the other + is off, then OK and, while any job is on, Disable all, and while any is off, Delete disabled jobs (edit), or Cancel + (run); β—€ and β–Ά to the other pages.""" if screen == 'top': possible = [('+', 'add', [])] + [(label, kind, args) for label, kind, args, there in ( @@ -136,6 +142,8 @@ def screen_buttons(screen, page, pages, shown, crc, own): rows = buttons.chunks([buttons.menu_button(f'{number} {job.name}', screen, [page, number], crc, buttons.tone(job.on) if screen == 'edit' else '') for number, job, _ in shown], JOB_ROW) nav = [buttons.menu_button('OK' if screen == 'edit' else 'Cancel', 'top', [0], crc)] + if screen == 'edit' and any(job.on for job in own): + nav.append(buttons.menu_button('Disable all', screen, [page, 'off'], crc)) if screen == 'edit' and not all(job.on for job in own): nav.append(buttons.menu_button('Delete disabled jobs', screen, [page, 'ask'], crc)) nav += buttons.page_buttons(page, pages, lambda label, to: buttons.menu_button(label, screen, [to], crc)) diff --git a/lib/worker.py b/lib/worker.py index 144caf0..71f4294 100755 --- a/lib/worker.py +++ b/lib/worker.py @@ -148,15 +148,21 @@ def run_now(chat, scheduler, job, number): delivery.deliver(chat, delivery.Report([about.job_line(chat.dialect, number, job, outcome)])) def edit_step(service, chat, scheduler, pressed, page): - """A press of the .jobs editing screen: a job switched on or off, the jobs that are off asked about (ask) or deleted - (yes), as long as the list is the one the button was drawn for (its checksum), else the screen is drawn as the list - is now, saying so.""" + """A press of the .jobs editing screen: a job switched on or off, all switched off (off), the jobs that are off + asked about (ask) or deleted (yes), as long as the list is the one the button was drawn for (its checksum), else + the screen is drawn as the list is now, saying so.""" own, what = scheduler.chat_jobs(chat), pressed.args[1] if jobs.list_checksum(own) != pressed.checksum: return about.jobs_screen(service, chat, 'edit', page, CHANGED) note = None if what == 'ask' and not all(job.on for job in own): return about.delete_disabled_screen(service, chat, page) + if what == 'off': + turned = [job for job in own if job.on] + for job in turned: + scheduler.toggle(job, on=False) + s = '' if len(turned) == 1 else 's' + return about.jobs_screen(service, chat, 'edit', page, f"Disabled {len(turned)} job{s}") if what == 'yes': gone = [job for job in own if not job.on] for job in gone: diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 2b340e7..915d0be 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -746,7 +746,7 @@ def test_enable_and_disable_a_job(self): with self.subTest(typed=typed): self.assertEqual(self.say(typed)[0], 'Tap a job to enable or disable it') self.assertEqual([[button.label for button in row] for row in self.replies[-1][2]], - [['1 rating', '2 movers close US', '3 trades'], ['OK', 'Delete disabled jobs']]) + [['1 rating', '2 movers close US', '3 trades'], ['OK', 'Disable all', 'Delete disabled jobs']]) def test_toggle_a_job_off_and_on(self): """.jobs toggle N switches job N off (it no longer fires) and on again.""" @@ -847,9 +847,9 @@ def tones(): """The tones of the buttons of the last screen, by row.""" return [[button.tone for button in row] for row in self.edits[-1][3]] self.press(f'm|edit|0|{self.crc()}') - self.assertEqual(tones(), [[on, on, on], ['']]) + self.assertEqual(tones(), [[on, on, on], ['', '']]) lines, rows, data = self.press(f'm|edit|0|1|{self.crc()}') - self.assertEqual(tones(), [[off, on, on], ['', '']]) + self.assertEqual(tones(), [[off, on, on], ['', '', '']]) self.press(f'm|run|0|{self.crc()}') self.assertEqual({tone for row in tones() for tone in row}, {''}) @@ -859,10 +859,10 @@ def test_edit_switches_jobs_and_deletes_the_disabled_after_asking(self): another list switch and delete nothing.""" lines, rows, data = self.press(f'm|edit|0|{self.crc()}') self.assertEqual((lines[0], rows), ('Tap a job to enable or disable it', [['1 rating', - '2 movers close US', '3 trades'], ['OK']])) + '2 movers close US', '3 trades'], ['OK', 'Disable all']])) lines, rows, data = self.press(data['1 rating']) self.assertEqual((lines[0], rows), ('Disabled 1. rating', [['1 rating', '2 movers close US', '3 trades'], - ['OK', 'Delete disabled jobs']])) + ['OK', 'Disable all', 'Delete disabled jobs']])) self.assertEqual(state.load(jobs.JOBS_FILE)[CHAT]['off'], ['rating']) lines, rows, data = self.press(data['Delete disabled jobs']) self.assertEqual((lines[0], rows), ('Delete the 1 disabled job?', [['Yes, delete them', 'Cancel']])) @@ -1128,7 +1128,8 @@ def fits(service, lines, markup): fits(service, *about.add_failed(service, CHAT, ['5', 'd', '9'], 'that job is here already')) pages = len({about.jobs_screen(DISCORD, CHAT, 'edit', page)[0][0] for page in range(6)}) self.assertEqual(pages, 4) # 64 jobs, 16 to a page on Discord - self.assertEqual(len({about.jobs_screen(TELEGRAM, CHAT, 'top', page)[0][0] for page in range(6)}), 2) + lines, markup = about.jobs_screen(DISCORD, CHAT, 'top', 3) # no pages: what fits, and how many more + self.assertEqual((lines[0], 'β–Ά' in str(markup), '…and ' in str(lines)), ('**Jobs here**', False, True)) class ReportImports(FinbotCase): """What importing the reports brings in.""" From 5a52ad02259eddfc24f2bfaa11e104264fda67d9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 00:08:32 +0000 Subject: [PATCH 12/25] jobs > Edit: Cancel button Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/scheduling.md | 2 +- lib/reports/about.py | 7 ++++--- tests/test_scheduler.py | 11 ++++++----- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/doc/scheduling.md b/doc/scheduling.md index 3cebdb4..365f7bd 100644 --- a/doc/scheduling.md +++ b/doc/scheduling.md @@ -22,7 +22,7 @@ never run, its name and its schedule in words. Under the list, in italics, is ho ``` Under it is one row of buttons, each there only where it has something to act on (**+** always; **Edit** and **Run** with any job, **Upcoming** with one that is on). Each screen is drawn in place of the last: * **+**: what the job runs, how often, then the rest down to the minute ([below](#add-step-by-step)). -* **Edit**: the list with a button per job, the default ones too; a press switches that job on or off at once. A job that is off is struck through in the list and has a red button (on Telegram, Slack and Discord alike); one that is on has a plain one. **OK** goes back to the list. **Disable all**, there while any job is on, switches them all off at once. **Delete disabled jobs**, there while any job is off, asks first ("Delete the 3 disabled jobs?"), then deletes them. A default job that is deleted comes back when the same job is added again. +* **Edit**: the list with a button per job, the default ones too; a press switches that job on or off at once. A job that is off is struck through in the list and has a red button (on Telegram, Slack and Discord alike); one that is on has a plain one. **OK** and **Cancel** both go back to the list (a switch is made at once, so Cancel undoes nothing). **Disable all**, there while any job is on, switches them all off at once. **Delete disabled jobs**, there while any job is off, asks first ("Delete the 3 disabled jobs?"), then deletes them. A default job that is deleted comes back when the same job is added again. * **Run**: a button per job; a press runs it now, as its schedule would. What the job posts shows it ran; a message says so only when it failed, or was running already. The same button pressed again within a minute does nothing more, as with every button. diff --git a/lib/reports/about.py b/lib/reports/about.py index 09fb322..72dc1ac 100644 --- a/lib/reports/about.py +++ b/lib/reports/about.py @@ -129,9 +129,8 @@ def paged(entries, service, with_buttons=True): def screen_buttons(screen, page, pages, shown, crc, own): """A list screen's buttons: top's, what can be done (+; Edit and Run with any job, Upcoming with one on), in one row; edit's and run's, a button for each job shown, ' ', JOB_ROW to a row, edit's red for a job that - is off, then OK and, while any job is on, Disable all, and while any is off, Delete disabled jobs (edit), or Cancel - (run); β—€ and β–Ά to the other - pages.""" + is off, then OK and, on edit, Cancel (both back to the list, as a switch is made at once), then, while any job is + on, Disable all, and while any is off, Delete disabled jobs; on run, Cancel; β—€ and β–Ά to the other pages.""" if screen == 'top': possible = [('+', 'add', [])] + [(label, kind, args) for label, kind, args, there in ( ('Edit', 'edit', [0], bool(own)), ('Run', 'run', [0], bool(own)), @@ -142,6 +141,8 @@ def screen_buttons(screen, page, pages, shown, crc, own): rows = buttons.chunks([buttons.menu_button(f'{number} {job.name}', screen, [page, number], crc, buttons.tone(job.on) if screen == 'edit' else '') for number, job, _ in shown], JOB_ROW) nav = [buttons.menu_button('OK' if screen == 'edit' else 'Cancel', 'top', [0], crc)] + if screen == 'edit': + nav.append(buttons.menu_button('Cancel', 'top', [0, 'x'], crc)) # its own data: a button's id is unique if screen == 'edit' and any(job.on for job in own): nav.append(buttons.menu_button('Disable all', screen, [page, 'off'], crc)) if screen == 'edit' and not all(job.on for job in own): diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 915d0be..0fed665 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -746,7 +746,8 @@ def test_enable_and_disable_a_job(self): with self.subTest(typed=typed): self.assertEqual(self.say(typed)[0], 'Tap a job to enable or disable it') self.assertEqual([[button.label for button in row] for row in self.replies[-1][2]], - [['1 rating', '2 movers close US', '3 trades'], ['OK', 'Disable all', 'Delete disabled jobs']]) + [['1 rating', '2 movers close US', '3 trades'], + ['OK', 'Cancel', 'Disable all', 'Delete disabled jobs']]) def test_toggle_a_job_off_and_on(self): """.jobs toggle N switches job N off (it no longer fires) and on again.""" @@ -847,9 +848,9 @@ def tones(): """The tones of the buttons of the last screen, by row.""" return [[button.tone for button in row] for row in self.edits[-1][3]] self.press(f'm|edit|0|{self.crc()}') - self.assertEqual(tones(), [[on, on, on], ['', '']]) + self.assertEqual(tones(), [[on, on, on], ['', '', '']]) lines, rows, data = self.press(f'm|edit|0|1|{self.crc()}') - self.assertEqual(tones(), [[off, on, on], ['', '', '']]) + self.assertEqual(tones(), [[off, on, on], ['', '', '', '']]) self.press(f'm|run|0|{self.crc()}') self.assertEqual({tone for row in tones() for tone in row}, {''}) @@ -859,10 +860,10 @@ def test_edit_switches_jobs_and_deletes_the_disabled_after_asking(self): another list switch and delete nothing.""" lines, rows, data = self.press(f'm|edit|0|{self.crc()}') self.assertEqual((lines[0], rows), ('Tap a job to enable or disable it', [['1 rating', - '2 movers close US', '3 trades'], ['OK', 'Disable all']])) + '2 movers close US', '3 trades'], ['OK', 'Cancel', 'Disable all']])) lines, rows, data = self.press(data['1 rating']) self.assertEqual((lines[0], rows), ('Disabled 1. rating', [['1 rating', '2 movers close US', '3 trades'], - ['OK', 'Disable all', 'Delete disabled jobs']])) + ['OK', 'Cancel', 'Disable all', 'Delete disabled jobs']])) self.assertEqual(state.load(jobs.JOBS_FILE)[CHAT]['off'], ['rating']) lines, rows, data = self.press(data['Delete disabled jobs']) self.assertEqual((lines[0], rows), ('Delete the 1 disabled job?', [['Yes, delete them', 'Cancel']])) From ef250374bc79e58dc72bdeb9885fda9fdc89f8a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 00:10:23 +0000 Subject: [PATCH 13/25] schedule log line after a change names the chat (network and id) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- lib/jobs.py | 15 ++++++++------- tests/test_schedule.py | 10 ++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/jobs.py b/lib/jobs.py index f0b63bd..41c0563 100644 --- a/lib/jobs.py +++ b/lib/jobs.py @@ -334,16 +334,17 @@ def __init__(self, catchup=None, clock=None, sleep=None, spawn=None): self.loading = threading.Lock() # one load at a time, so the list swapped in last is the one read last self.saving = threading.Lock() # one save of the record of runs at a time, so the one written last is newest - def load(self): + def load(self, changed=None): """Read every job: the system jobs; for each chat JOBS_FILE knows, the catalog's (switched off where its record says so) and those it added. A stored job that cannot be (an action that is none, arguments its parser refuses: an edited or older file) is logged and left out, never run. A job kept from before keeps its timing; a new one counts from now, as one does that was off. Loads take turns: one reading the file while another change is stored - would otherwise swap in the older list after the newer one.""" + would otherwise swap in the older list after the newer one. `changed` is the chat (its key) whose change or + first sight this reads for, said in the log.""" with self.loading: - self._load() + self._load(changed) - def _load(self): + def _load(self, changed=None): """load(), with the loading lock held.""" jobs = system_jobs() chats = self.stored_chats() @@ -383,7 +384,7 @@ def _load(self): is_dm(key)) log.info(f"schedule: {len(jobs) - len(SYSTEM)} jobs of {len(chats) - dms} group" f" chat{'' if len(chats) - dms == 1 else 's'} and {dms} DM{'' if dms == 1 else 's'}, and {len(SYSTEM)}" - " system jobs") + f" system jobs{f' (for {changed})' if changed else ''}") def stored_chats(self): """The chats JOBS_FILE knows, each with its record, made current in the file (clean) where an older finbot left @@ -543,7 +544,7 @@ def see(self, chat, dm=None): known = chat in self.chats if not known: know([chat], dm) - self.load() + self.load(chat) def change(self, chat, edit): """Change a chat's jobs as stored, and return what edit(record) returned: it changes the chat's record ({'off': @@ -554,7 +555,7 @@ def change(self, chat, edit): if not isinstance(record.get('jobs', []), list): record['jobs'] = [] result = edit(record) - self.load() + self.load(chat) return result def add(self, chat, action, args, schedule): diff --git a/tests/test_schedule.py b/tests/test_schedule.py index 48fc7d2..43ac5ae 100644 --- a/tests/test_schedule.py +++ b/tests/test_schedule.py @@ -328,6 +328,16 @@ def test_the_schedule_counts_group_chats_and_dms(self): jobs.Scheduler(clock=lambda: OCTOBER).load() self.assertIn('schedule: 0 jobs of 2 group chats and 1 DM, and 0 system jobs', log.getvalue()) + def test_the_schedule_line_names_the_chat_a_change_was_for(self): + """A load that follows a change to a chat's jobs says which chat, by network and id, in its log line.""" + self.patch(catalog, 'ITEMS', ()) + self.patch(jobs, 'SYSTEM', ()) + state.save(jobs.JOBS_FILE, {'slack:C1': {}}) + scheduler = jobs.Scheduler(clock=lambda: OCTOBER) + with contextlib.redirect_stderr(io.StringIO()) as log: + scheduler.change('slack:C1', lambda record: None) + self.assertIn('0 system jobs (for slack:C1)', log.getvalue()) + if __name__ == '__main__': unittest.main() From 8c998206f289456e888f903b6d4210962c134e91 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 00:14:02 +0000 Subject: [PATCH 14/25] jobs > Edit: Select all / Select none, Delete and OK only with a selection; Run becomes Run... Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/scheduling.md | 6 +++--- lib/reports/about.py | 21 +++++++++++++-------- lib/worker.py | 12 +++++++++--- tests/test_scheduler.py | 32 +++++++++++++++++++------------- 4 files changed, 44 insertions(+), 27 deletions(-) diff --git a/doc/scheduling.md b/doc/scheduling.md index 365f7bd..e89099b 100644 --- a/doc/scheduling.md +++ b/doc/scheduling.md @@ -20,10 +20,10 @@ never run, its name and its schedule in words. Under the list, in italics, is ho /jobs add REPORT SCHEDULE /jobs add reminder SCHEDULE "TEXT" ``` -Under it is one row of buttons, each there only where it has something to act on (**+** always; **Edit** and **Run** with any job, **Upcoming** with one that is on). Each screen is drawn in place of the last: +Under it is one row of buttons, each there only where it has something to act on (**+** always; **Edit** and **Run...** with any job, **Upcoming** with one that is on). Each screen is drawn in place of the last: * **+**: what the job runs, how often, then the rest down to the minute ([below](#add-step-by-step)). -* **Edit**: the list with a button per job, the default ones too; a press switches that job on or off at once. A job that is off is struck through in the list and has a red button (on Telegram, Slack and Discord alike); one that is on has a plain one. **OK** and **Cancel** both go back to the list (a switch is made at once, so Cancel undoes nothing). **Disable all**, there while any job is on, switches them all off at once. **Delete disabled jobs**, there while any job is off, asks first ("Delete the 3 disabled jobs?"), then deletes them. A default job that is deleted comes back when the same job is added again. -* **Run**: a button per job; a press runs it now, as its schedule would. What the job posts shows it ran; a +* **Edit**: the list with a button per job, the default ones too; a press switches that job on or off at once. A job that is off is struck through in the list and has a red button (on Telegram, Slack and Discord alike); one that is on has a plain one. **Select all** switches every job off at once (all selected); it reads **Select none** once all are off, and switches them all on. While any job is off (selected), **Delete** asks first ("Delete the 3 disabled jobs?"), then deletes them, and **OK** goes back to the list. **Cancel** goes back to the list too (a switch is made at once, so Cancel undoes nothing). A default job that is deleted comes back when the same job is added again. +* **Run...**: a button per job; a press runs it now, as its schedule would. What the job posts shows it ran; a message says so only when it failed, or was running already. The same button pressed again within a minute does nothing more, as with every button. * **Upcoming**: the next runs, in time order. diff --git a/lib/reports/about.py b/lib/reports/about.py index 72dc1ac..4d0fcb3 100644 --- a/lib/reports/about.py +++ b/lib/reports/about.py @@ -129,24 +129,29 @@ def paged(entries, service, with_buttons=True): def screen_buttons(screen, page, pages, shown, crc, own): """A list screen's buttons: top's, what can be done (+; Edit and Run with any job, Upcoming with one on), in one row; edit's and run's, a button for each job shown, ' ', JOB_ROW to a row, edit's red for a job that - is off, then OK and, on edit, Cancel (both back to the list, as a switch is made at once), then, while any job is - on, Disable all, and while any is off, Delete disabled jobs; on run, Cancel; β—€ and β–Ά to the other pages.""" + is off (selected), then on edit Select all (Select none where all are selected) and, while any is selected, Delete + and OK, then Cancel (OK and Cancel go back to the list: a switch is made at once); on run, Cancel; β—€ and β–Ά to the + other pages.""" if screen == 'top': possible = [('+', 'add', [])] + [(label, kind, args) for label, kind, args, there in ( - ('Edit', 'edit', [0], bool(own)), ('Run', 'run', [0], bool(own)), + ('Edit', 'edit', [0], bool(own)), ('Run...', 'run', [0], bool(own)), ('Upcoming', 'next', [], any(job.on for job in own))) if there] rows = [[buttons.menu_button(label, kind, args, crc) for label, kind, args in possible]] nav = [] else: # edit or run: a button per job rows = buttons.chunks([buttons.menu_button(f'{number} {job.name}', screen, [page, number], crc, buttons.tone(job.on) if screen == 'edit' else '') for number, job, _ in shown], JOB_ROW) - nav = [buttons.menu_button('OK' if screen == 'edit' else 'Cancel', 'top', [0], crc)] + nav = [] if screen == 'edit': + everything = not any(job.on for job in own) # all selected: the jobs that are off are the ones selected + nav.append(buttons.menu_button('Select none' if everything else 'Select all', screen, + [page, 'on' if everything else 'off'], crc)) + if not all(job.on for job in own): # something selected + nav.append(buttons.menu_button('Delete', screen, [page, 'ask'], crc)) + nav.append(buttons.menu_button('OK', 'top', [0], crc)) nav.append(buttons.menu_button('Cancel', 'top', [0, 'x'], crc)) # its own data: a button's id is unique - if screen == 'edit' and any(job.on for job in own): - nav.append(buttons.menu_button('Disable all', screen, [page, 'off'], crc)) - if screen == 'edit' and not all(job.on for job in own): - nav.append(buttons.menu_button('Delete disabled jobs', screen, [page, 'ask'], crc)) + else: + nav.append(buttons.menu_button('Cancel', 'top', [0], crc)) nav += buttons.page_buttons(page, pages, lambda label, to: buttons.menu_button(label, screen, [to], crc)) return rows + ([nav] if nav else []) diff --git a/lib/worker.py b/lib/worker.py index 71f4294..8398aa9 100755 --- a/lib/worker.py +++ b/lib/worker.py @@ -148,15 +148,21 @@ def run_now(chat, scheduler, job, number): delivery.deliver(chat, delivery.Report([about.job_line(chat.dialect, number, job, outcome)])) def edit_step(service, chat, scheduler, pressed, page): - """A press of the .jobs editing screen: a job switched on or off, all switched off (off), the jobs that are off - asked about (ask) or deleted (yes), as long as the list is the one the button was drawn for (its checksum), else - the screen is drawn as the list is now, saying so.""" + """A press of the .jobs editing screen: a job switched on or off, all switched off (off) or on (on), the jobs that + are off asked about (ask) or deleted (yes), as long as the list is the one the button was drawn for (its + checksum), else the screen is drawn as the list is now, saying so.""" own, what = scheduler.chat_jobs(chat), pressed.args[1] if jobs.list_checksum(own) != pressed.checksum: return about.jobs_screen(service, chat, 'edit', page, CHANGED) note = None if what == 'ask' and not all(job.on for job in own): return about.delete_disabled_screen(service, chat, page) + if what == 'on': + turned = [job for job in own if not job.on] + for job in turned: + scheduler.toggle(job, on=True) + s = '' if len(turned) == 1 else 's' + return about.jobs_screen(service, chat, 'edit', page, f"Enabled {len(turned)} job{s}") if what == 'off': turned = [job for job in own if job.on] for job in turned: diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 0fed665..d884947 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -580,7 +580,7 @@ def test_add_list_and_remove(self): '* not run yet', '', '/jobs add REPORT SCHEDULE', '/jobs add reminder SCHEDULE "TEXT"']) self.assertEqual([[button.label for button in row] for row in self.replies[-1][2]], - [['+', 'Edit', 'Run', 'Upcoming']]) + [['+', 'Edit', 'Run...', 'Upcoming']]) self.assertEqual(self.say('.jobs del 4'), ['Deleted 4. trades Β· every 20 mins, weekdays UTC']) shorts = entry('shorts daily at 10pm', zone='Australia/Melbourne') self.assertEqual(state.load(jobs.JOBS_FILE)[CHAT], {'jobs': [shorts]}) @@ -747,7 +747,7 @@ def test_enable_and_disable_a_job(self): self.assertEqual(self.say(typed)[0], 'Tap a job to enable or disable it') self.assertEqual([[button.label for button in row] for row in self.replies[-1][2]], [['1 rating', '2 movers close US', '3 trades'], - ['OK', 'Cancel', 'Disable all', 'Delete disabled jobs']]) + ['Select all', 'Delete', 'OK', 'Cancel']]) def test_toggle_a_job_off_and_on(self): """.jobs toggle N switches job N off (it no longer fires) and on again.""" @@ -848,7 +848,7 @@ def tones(): """The tones of the buttons of the last screen, by row.""" return [[button.tone for button in row] for row in self.edits[-1][3]] self.press(f'm|edit|0|{self.crc()}') - self.assertEqual(tones(), [[on, on, on], ['', '', '']]) + self.assertEqual(tones(), [[on, on, on], ['', '']]) lines, rows, data = self.press(f'm|edit|0|1|{self.crc()}') self.assertEqual(tones(), [[off, on, on], ['', '', '', '']]) self.press(f'm|run|0|{self.crc()}') @@ -856,26 +856,32 @@ def tones(): def test_edit_switches_jobs_and_deletes_the_disabled_after_asking(self): """Edit lists the jobs with a button for each, red where it is off: a press switches that job, OK - goes back, and Delete disabled jobs, shown while one is off, asks before it deletes them; buttons drawn for - another list switch and delete nothing.""" + goes back, and Delete and OK, shown while one is off (selected), Select all switching every job off and then + reading Select none, which switches them on; Delete asks before it deletes them; buttons drawn for another + list switch and delete nothing.""" lines, rows, data = self.press(f'm|edit|0|{self.crc()}') self.assertEqual((lines[0], rows), ('Tap a job to enable or disable it', [['1 rating', - '2 movers close US', '3 trades'], ['OK', 'Cancel', 'Disable all']])) + '2 movers close US', '3 trades'], ['Select all', 'Cancel']])) lines, rows, data = self.press(data['1 rating']) self.assertEqual((lines[0], rows), ('Disabled 1. rating', [['1 rating', '2 movers close US', '3 trades'], - ['OK', 'Cancel', 'Disable all', 'Delete disabled jobs']])) + ['Select all', 'Delete', 'OK', 'Cancel']])) self.assertEqual(state.load(jobs.JOBS_FILE)[CHAT]['off'], ['rating']) - lines, rows, data = self.press(data['Delete disabled jobs']) + lines, rows, data = self.press(data['Select all']) + self.assertEqual((lines[0], rows[-1]), ('Disabled 2 jobs', ['Select none', 'Delete', 'OK', 'Cancel'])) + lines, rows, data = self.press(data['Select none']) + self.assertEqual((lines[0], rows[-1]), ('Enabled 3 jobs', ['Select all', 'Cancel'])) + lines, rows, data = self.press(data['1 rating']) + lines, rows, data = self.press(data['Delete']) self.assertEqual((lines[0], rows), ('Delete the 1 disabled job?', [['Yes, delete them', 'Cancel']])) self.assertEqual(lines[2], '1. ❌ rating') lines, rows, data = self.press(data['Cancel']) self.assertEqual(lines[0], 'Tap a job to enable or disable it') lines, rows, data = self.press(data['OK']) - self.assertEqual((lines[0], rows[0]), ('Jobs here', ['+', 'Edit', 'Run', 'Upcoming'])) + self.assertEqual((lines[0], rows[0]), ('Jobs here', ['+', 'Edit', 'Run...', 'Upcoming'])) self.press(f'm|edit|0|{self.crc()}') self.assertEqual(self.press('m|edit|0|ask|0000')[0][0], 'The jobs have changed: here they are now') lines, rows, data = self.press(f'm|edit|0|{self.crc()}') - lines, rows, data = self.press(data['Delete disabled jobs']) + lines, rows, data = self.press(data['Delete']) lines, rows, data = self.press(data['Yes, delete them']) self.assertEqual((lines[0], [job.name for job in self.s.chat_jobs(CHAT)]), ('Deleted 1 disabled job', ['movers close US', 'trades'])) @@ -886,10 +892,10 @@ def test_the_top_level_offers_only_what_can_be_done(self): def top(): """The top screen's button labels, by row.""" return [[button.label for button in row] for row in about.jobs_screen(TELEGRAM, CHAT)[1]] - self.assertEqual(top(), [['+', 'Edit', 'Run', 'Upcoming']]) + self.assertEqual(top(), [['+', 'Edit', 'Run...', 'Upcoming']]) for job in self.s.chat_jobs(CHAT): self.s.toggle(job, False) - self.assertEqual(top(), [['+', 'Edit', 'Run']]) + self.assertEqual(top(), [['+', 'Edit', 'Run...']]) for job in self.s.chat_jobs(CHAT): self.s.delete(job) self.assertEqual(top(), [['+']]) @@ -956,7 +962,7 @@ def test_add_by_buttons_back_and_cancel(self): self.assertEqual((rows[0], rows[-1]), (['14:00†', '14:05', '14:10', '14:15'], ['Back', 'Cancel'])) lines, rows, data = self.press(data['14:30']) self.assertEqual(newest(lines), '3. * movers Β· weekly, Fridays at 2:30pm') - self.assertEqual((lines[0], rows, data['+']), ('Added', [['+', 'Edit', 'Run', 'Upcoming']], + self.assertEqual((lines[0], rows, data['+']), ('Added', [['+', 'Edit', 'Run...', 'Upcoming']], f'm|add|{self.crc()}')) # the whole list, the job added in bold, with the list's buttons self.assertEqual(state.load(jobs.JOBS_FILE)[CHAT]['jobs'][-1], entry('movers every friday at 2:30pm', zone='Australia/Melbourne')) From 36a62de0476aa94e9c63f55b03ac18119339f171 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 00:20:37 +0000 Subject: [PATCH 15/25] jobs > Edit selects jobs and changes nothing until OK, which shows only when the selection differs; schedule log says ': updated' Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/scheduling.md | 2 +- lib/jobs.py | 6 ++- lib/reports/about.py | 83 ++++++++++++++++++++++++----------------- lib/worker.py | 58 ++++++++++++++-------------- tests/test_schedule.py | 4 +- tests/test_scheduler.py | 71 ++++++++++++++++++++--------------- 6 files changed, 123 insertions(+), 101 deletions(-) diff --git a/doc/scheduling.md b/doc/scheduling.md index e89099b..24d4514 100644 --- a/doc/scheduling.md +++ b/doc/scheduling.md @@ -22,7 +22,7 @@ never run, its name and its schedule in words. Under the list, in italics, is ho ``` Under it is one row of buttons, each there only where it has something to act on (**+** always; **Edit** and **Run...** with any job, **Upcoming** with one that is on). Each screen is drawn in place of the last: * **+**: what the job runs, how often, then the rest down to the minute ([below](#add-step-by-step)). -* **Edit**: the list with a button per job, the default ones too; a press switches that job on or off at once. A job that is off is struck through in the list and has a red button (on Telegram, Slack and Discord alike); one that is on has a plain one. **Select all** switches every job off at once (all selected); it reads **Select none** once all are off, and switches them all on. While any job is off (selected), **Delete** asks first ("Delete the 3 disabled jobs?"), then deletes them, and **OK** goes back to the list. **Cancel** goes back to the list too (a switch is made at once, so Cancel undoes nothing). A default job that is deleted comes back when the same job is added again. +* **Edit**: the list with a button per job, the default ones too; a press selects the job (its button turns red, on Telegram, Slack and Discord alike) or takes it out of the selection again. It starts with the jobs that are off selected, and nothing changes until **OK**, which switches the selected jobs off and the rest on. **OK** shows only where the selection differs from the jobs as they are, and **Delete** only while any job is selected; **Delete** asks first ("Delete the 3 selected jobs?"), then deletes them. **Select all** selects every job, and reads **Select none** once all are selected. **Cancel** goes back to the list, whatever was selected. A default job that is deleted comes back when the same job is added again. * **Run...**: a button per job; a press runs it now, as its schedule would. What the job posts shows it ran; a message says so only when it failed, or was running already. The same button pressed again within a minute does nothing more, as with every button. diff --git a/lib/jobs.py b/lib/jobs.py index 41c0563..6b01259 100644 --- a/lib/jobs.py +++ b/lib/jobs.py @@ -340,7 +340,7 @@ def load(self, changed=None): an edited or older file) is logged and left out, never run. A job kept from before keeps its timing; a new one counts from now, as one does that was off. Loads take turns: one reading the file while another change is stored would otherwise swap in the older list after the newer one. `changed` is the chat (its key) whose change or - first sight this reads for, said in the log.""" + first sight this reads for: the log then says only that it was updated.""" with self.loading: self._load(changed) @@ -380,11 +380,13 @@ def _load(self, changed=None): self.save_runs() if problem: delivery.report_error(ScheduleError(problem), context='schedule') + if changed: + return log.info(f"schedule: {changed}: updated") dms = sum(1 for key, record in chats.items() if isinstance(record, dict) and record.get('dm') is True or is_dm(key)) log.info(f"schedule: {len(jobs) - len(SYSTEM)} jobs of {len(chats) - dms} group" f" chat{'' if len(chats) - dms == 1 else 's'} and {dms} DM{'' if dms == 1 else 's'}, and {len(SYSTEM)}" - f" system jobs{f' (for {changed})' if changed else ''}") + " system jobs") def stored_chats(self): """The chats JOBS_FILE knows, each with its record, made current in the file (clean) where an older finbot left diff --git a/lib/reports/about.py b/lib/reports/about.py index 4d0fcb3..ddadb7e 100644 --- a/lib/reports/about.py +++ b/lib/reports/about.py @@ -11,7 +11,8 @@ from lib import sharesight from lib import when -HEADINGS = {'top': 'Jobs here', 'edit': 'Tap a job to enable or disable it', 'run': 'Tap a job to run it now'} +HEADINGS = {'top': 'Jobs here', 'edit': 'Select jobs to disable or delete (the rest are enabled)', + 'run': 'Tap a job to run it now'} ADD_TYPED = ['/jobs add REPORT SCHEDULE', '/jobs add reminder SCHEDULE "TEXT"'] # how Add is typed: under the list NOT_RUN = '* not run yet' # the key to a list's mark PAGE_ROOM = 500 # characters of a screen besides its jobs' lines: its headings, a note, the key to * @@ -43,14 +44,15 @@ YEARS = 10 # the years a one-off may be in, this one first CROWDED = 20 # a screen of more choices takes them in rows of five, Back last, on every network: Discord holds 25 -def jobs_screen(service, chat, screen='top', page=0, note=None, highlight=None): +def jobs_screen(service, chat, screen='top', page=0, note=None, highlight=None, selected=None): """A screen of the .jobs menu that lists a chat's jobs (by its key), numbered, each struck through and marked ❌ when it is off, * where it has never run, its name and its schedule in words, by the timezone setting's clock (lib/when.py), then the key to the marks shown. top has what can be done in one row (+ to add, and where there is a job to act on, Edit, Run, Upcoming) and how + is typed, in italics; edit and run a button for each job, which switches it on or off, or runs it now. A page at a time, as many jobs as a message of the network takes (β—€ and β–Ά for the others), after a note of what the last press did. A one-off gone by is deleted first. Returns (lines, - buttons). The job numbered `highlight` is in bold with a πŸ‘‰ before it, on its page.""" + buttons). The job numbered `highlight` is in bold with a πŸ‘‰ before it, on its page. On edit, `selected` is the jobs + selected (a bit each, by number less one: see selection; the jobs that are off now when None).""" scheduler = jobs.current scheduler.prune(chat) own = scheduler.chat_jobs(chat) @@ -79,7 +81,8 @@ def jobs_screen(service, chat, screen='top', page=0, note=None, highlight=None): lines += ['', NOT_RUN] if screen == 'top': lines += ['', *(service.italic(service.escape(line)) for line in ADD_TYPED)] - return lines, screen_buttons(screen, page, len(parts), shown, jobs.list_checksum(own), own) + chosen = selection(own) if selected is None else selected + return lines, screen_buttons(screen, page, len(parts), shown, jobs.list_checksum(own), own, chosen) def grouped(service, shown): """The entries (number, job, line) as lines: the jobs of each report, or the reminders, under a heading of their @@ -126,45 +129,57 @@ def paged(entries, service, with_buttons=True): size += len(entry[2]) + 1 return parts -def screen_buttons(screen, page, pages, shown, crc, own): - """A list screen's buttons: top's, what can be done (+; Edit and Run with any job, Upcoming with one on), in one - row; edit's and run's, a button for each job shown, ' ', JOB_ROW to a row, edit's red for a job that - is off (selected), then on edit Select all (Select none where all are selected) and, while any is selected, Delete - and OK, then Cancel (OK and Cancel go back to the list: a switch is made at once); on run, Cancel; β—€ and β–Ά to the - other pages.""" +def selection(own): + """The selection that matches the jobs as they are, Edit's start: a bit for each job (its number less one) that + is off.""" + return sum(1 << i for i, job in enumerate(own) if not job.on) + +def screen_buttons(screen, page, pages, shown, crc, own, selected=0): + """A list screen's buttons: top's, what can be done (+; Edit and Run... with any job, Upcoming with one on), in one + row; edit's and run's, a button for each job shown, ' '. On edit a job's button selects the job or + not (red when selected: the selection, `selected`, is in each button's data, and nothing changes until OK), + JOB_ROW to a row, then Select all (Select none where all are selected), while any is selected Delete, while the + selection differs from the jobs as they are OK (it switches the selected jobs off and the rest on), and Cancel, + back to the list; run's Cancel; β—€ and β–Ά to the other pages.""" if screen == 'top': possible = [('+', 'add', [])] + [(label, kind, args) for label, kind, args, there in ( ('Edit', 'edit', [0], bool(own)), ('Run...', 'run', [0], bool(own)), ('Upcoming', 'next', [], any(job.on for job in own))) if there] rows = [[buttons.menu_button(label, kind, args, crc) for label, kind, args in possible]] nav = [] - else: # edit or run: a button per job - rows = buttons.chunks([buttons.menu_button(f'{number} {job.name}', screen, [page, number], crc, - buttons.tone(job.on) if screen == 'edit' else '') for number, job, _ in shown], JOB_ROW) - nav = [] - if screen == 'edit': - everything = not any(job.on for job in own) # all selected: the jobs that are off are the ones selected - nav.append(buttons.menu_button('Select none' if everything else 'Select all', screen, - [page, 'on' if everything else 'off'], crc)) - if not all(job.on for job in own): # something selected - nav.append(buttons.menu_button('Delete', screen, [page, 'ask'], crc)) - nav.append(buttons.menu_button('OK', 'top', [0], crc)) - nav.append(buttons.menu_button('Cancel', 'top', [0, 'x'], crc)) # its own data: a button's id is unique - else: - nav.append(buttons.menu_button('Cancel', 'top', [0], crc)) - nav += buttons.page_buttons(page, pages, lambda label, to: buttons.menu_button(label, screen, [to], crc)) + elif screen == 'run': + rows = buttons.chunks([buttons.menu_button(f'{number} {job.name}', 'run', [page, number], crc) + for number, job, _ in shown], JOB_ROW) + nav = [buttons.menu_button('Cancel', 'top', [0], crc)] + else: # edit: the selection travels in each button's data, as a picker's ticks do + full = (1 << len(own)) - 1 + def pick(to, what, mask): + """The arguments of a press of edit: the page to show, what it asks for and the selection it makes.""" + return [to, what, format(mask, 'x')] + rows = buttons.chunks([buttons.menu_button(f'{number} {job.name}', 'edit', + pick(page, 'm', selected ^ (1 << number - 1)), crc, buttons.tone(not selected >> number - 1 & 1)) + for number, job, _ in shown], JOB_ROW) + nav = [buttons.menu_button('Select none' if selected == full else 'Select all', 'edit', + pick(page, 'm', 0 if selected == full else full), crc)] + if selected: + nav.append(buttons.menu_button('Delete', 'edit', pick(page, 'ask', selected), crc)) + if selected != selection(own): + nav.append(buttons.menu_button('OK', 'edit', pick(page, 'ok', selected), crc)) + nav.append(buttons.menu_button('Cancel', 'top', [0], crc)) + nav += buttons.page_buttons(page, pages, lambda label, to: buttons.menu_button(label, screen, + pick(to, 'm', selected) if screen == 'edit' else [to], crc)) return rows + ([nav] if nav else []) -def delete_disabled_screen(service, chat, page=0): - """Delete disabled jobs' question, for a chat (by its key): whether to delete the jobs that are off, listed, with - Yes, delete them and Cancel, back to the editing.""" +def delete_screen(service, chat, selected, page=0): + """The question of deleting the selected jobs, for a chat (by its key), listed, with Yes, delete them and Cancel, + back to the editing with the selection kept.""" own = jobs.current.chat_jobs(chat) - off = [(number, job) for number, job in enumerate(own, 1) if not job.on] - crc = jobs.list_checksum(own) - return [service.bold(f"Delete the {len(off)} disabled job{'' if len(off) == 1 else 's'}?"), '', - *(f"{number}. ❌ {service.strike(service.escape(job.name))}" for number, job in off)], [[ - buttons.menu_button('Yes, delete them', 'edit', [page, 'yes'], crc), - buttons.menu_button('Cancel', 'edit', [page], crc)]] + chosen = [(number, job) for number, job in enumerate(own, 1) if selected >> number - 1 & 1] + crc, kept = jobs.list_checksum(own), format(selected, 'x') + return [service.bold(f"Delete the {len(chosen)} selected job{'' if len(chosen) == 1 else 's'}?"), '', + *(f"{number}. {service.escape(job.name)}" for number, job in chosen)], [[ + buttons.menu_button('Yes, delete them', 'edit', [page, 'yes', kept], crc), + buttons.menu_button('Cancel', 'edit', [page, 'm', kept], crc)]] def job_entry(service, scheduler, number, job, running=(), local=None, now=None): """'5. movers close AU Β· weekdays at 4:30pm Sydney': a job in .jobs, with its number, * where it has never run, its diff --git a/lib/worker.py b/lib/worker.py index 8398aa9..e46c607 100755 --- a/lib/worker.py +++ b/lib/worker.py @@ -130,7 +130,7 @@ def menu_screen(service, chat, pressed): if screen == 'add': return add_step(service, chat, args) page = int(args[0]) if args and args[0].isdigit() and len(args[0]) < 4 else 0 - if screen == 'edit' and len(args) == 2: + if screen == 'edit' and len(args) == 3 and all(arg.isalnum() for arg in args): return edit_step(service, key, scheduler, pressed, page) note = None if screen == 'run' and len(args) == 2: @@ -148,39 +148,35 @@ def run_now(chat, scheduler, job, number): delivery.deliver(chat, delivery.Report([about.job_line(chat.dialect, number, job, outcome)])) def edit_step(service, chat, scheduler, pressed, page): - """A press of the .jobs editing screen: a job switched on or off, all switched off (off) or on (on), the jobs that - are off asked about (ask) or deleted (yes), as long as the list is the one the button was drawn for (its - checksum), else the screen is drawn as the list is now, saying so.""" - own, what = scheduler.chat_jobs(chat), pressed.args[1] - if jobs.list_checksum(own) != pressed.checksum: + """A press of the .jobs editing screen, which carries the selection (a bit for each job: the jobs to disable, or to + delete) in its data: the screen again with that selection (m), the jobs switched to it (ok: the selected off, the + rest on), the selected jobs asked about (ask) or deleted (yes), as long as the list is the one the button was + drawn for (its checksum), else the screen is drawn as the list is now, saying so.""" + own, what, mask = scheduler.chat_jobs(chat), pressed.args[1], pressed.args[2] + selected = int(mask, 16) if all(c in '0123456789abcdef' for c in mask) else -1 + if jobs.list_checksum(own) != pressed.checksum or not 0 <= selected < 1 << len(own): return about.jobs_screen(service, chat, 'edit', page, CHANGED) - note = None - if what == 'ask' and not all(job.on for job in own): - return about.delete_disabled_screen(service, chat, page) - if what == 'on': - turned = [job for job in own if not job.on] - for job in turned: - scheduler.toggle(job, on=True) - s = '' if len(turned) == 1 else 's' - return about.jobs_screen(service, chat, 'edit', page, f"Enabled {len(turned)} job{s}") - if what == 'off': - turned = [job for job in own if job.on] - for job in turned: - scheduler.toggle(job, on=False) - s = '' if len(turned) == 1 else 's' - return about.jobs_screen(service, chat, 'edit', page, f"Disabled {len(turned)} job{s}") + chosen = [job for i, job in enumerate(own) if selected >> i & 1] + if what == 'ask' and chosen: + return about.delete_screen(service, chat, selected, page) + if what == 'ok': + off = on = 0 + try: # each set as stored, under the scheduler's lock + for i, job in enumerate(own): + if bool(selected >> i & 1) == job.on: + scheduler.toggle(job, on=not selected >> i & 1) + off, on = off + bool(selected >> i & 1), on + (not selected >> i & 1) + except jobs.ScheduleError as e: + return about.jobs_screen(service, chat, 'edit', page, service.escape(str(e))) + said = [f"{how} {count} job{'' if count == 1 else 's'}" for count, how in ((off, 'disabled'), (on, 'enabled')) + if count] + return about.jobs_screen(service, chat, 'top', 0, ', '.join(said).capitalize() if said else 'Nothing changed') if what == 'yes': - gone = [job for job in own if not job.on] - for job in gone: + for job in chosen: scheduler.delete(job) - s = '' if len(gone) == 1 else 's' - return about.jobs_screen(service, chat, 'top', 0, f"Deleted {len(gone)} disabled job{s}") - if job := jobs.numbered(own, what): - try: # flipped as stored, under the scheduler's lock: two presses at once flip it twice - note = f"{'Enabled' if scheduler.toggle(job) else 'Disabled'} {what}. {service.escape(job.name)}" - except jobs.ScheduleError as e: - note = service.escape(str(e)) - return about.jobs_screen(service, chat, 'edit', page, note) + s = '' if len(chosen) == 1 else 's' + return about.jobs_screen(service, chat, 'top', 0, f"Deleted {len(chosen)} job{s}") + return about.jobs_screen(service, chat, 'edit', page, selected=selected) def add_step(service, chat, steps): """A step of Add pressed: the next choice's screen; once every choice is made, the job added, or why it is not.""" diff --git a/tests/test_schedule.py b/tests/test_schedule.py index 43ac5ae..118830d 100644 --- a/tests/test_schedule.py +++ b/tests/test_schedule.py @@ -329,14 +329,14 @@ def test_the_schedule_counts_group_chats_and_dms(self): self.assertIn('schedule: 0 jobs of 2 group chats and 1 DM, and 0 system jobs', log.getvalue()) def test_the_schedule_line_names_the_chat_a_change_was_for(self): - """A load that follows a change to a chat's jobs says which chat, by network and id, in its log line.""" + """A load that follows a change to a chat's jobs logs which chat, by network and id, was updated.""" self.patch(catalog, 'ITEMS', ()) self.patch(jobs, 'SYSTEM', ()) state.save(jobs.JOBS_FILE, {'slack:C1': {}}) scheduler = jobs.Scheduler(clock=lambda: OCTOBER) with contextlib.redirect_stderr(io.StringIO()) as log: scheduler.change('slack:C1', lambda record: None) - self.assertIn('0 system jobs (for slack:C1)', log.getvalue()) + self.assertIn('schedule: slack:C1: updated', log.getvalue()) if __name__ == '__main__': diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index d884947..5aaf13c 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -744,10 +744,9 @@ def test_enable_and_disable_a_job(self): self.say('.jobs disable 1') for typed in ('.jobs enable', '.jobs disable', '.jobs delete'): with self.subTest(typed=typed): - self.assertEqual(self.say(typed)[0], 'Tap a job to enable or disable it') + self.assertEqual(self.say(typed)[0], 'Select jobs to disable or delete (the rest are enabled)') self.assertEqual([[button.label for button in row] for row in self.replies[-1][2]], - [['1 rating', '2 movers close US', '3 trades'], - ['Select all', 'Delete', 'OK', 'Cancel']]) + [['1 rating', '2 movers close US', '3 trades'], ['Select all', 'Delete', 'Cancel']]) def test_toggle_a_job_off_and_on(self): """.jobs toggle N switches job N off (it no longer fires) and on again.""" @@ -841,50 +840,59 @@ class JobsMenu(ChatBase): per job; Add's steps; a stale or forged press acting on nothing.""" def test_a_job_that_is_off_has_a_red_button(self): - """Edit draws a job's button red when the job is off and plain when it is on; the run list, not a toggle, is - plain.""" + """Edit draws a job's button red when the job is selected (off to start with) and plain when it is not; the + run list, not a toggle, is plain.""" off, on = buttons.OFF, '' def tones(): """The tones of the buttons of the last screen, by row.""" return [[button.tone for button in row] for row in self.edits[-1][3]] - self.press(f'm|edit|0|{self.crc()}') + lines, rows, data = self.press(f'm|edit|0|{self.crc()}') self.assertEqual(tones(), [[on, on, on], ['', '']]) - lines, rows, data = self.press(f'm|edit|0|1|{self.crc()}') + self.press(data['1 rating']) self.assertEqual(tones(), [[off, on, on], ['', '', '', '']]) self.press(f'm|run|0|{self.crc()}') self.assertEqual({tone for row in tones() for tone in row}, {''}) - def test_edit_switches_jobs_and_deletes_the_disabled_after_asking(self): - """Edit lists the jobs with a button for each, red where it is off: a press switches that job, OK - goes back, and Delete and OK, shown while one is off (selected), Select all switching every job off and then - reading Select none, which switches them on; Delete asks before it deletes them; buttons drawn for another - list switch and delete nothing.""" + def test_edit_selects_jobs_and_changes_nothing_until_ok(self): + """Edit's buttons select jobs (red) and change nothing until OK, which switches the selected off and the rest + on; OK shows only where the selection differs from the jobs as they are, Delete only with a selection, and + Select all reads Select none once all are selected. Delete asks before it deletes the selected; buttons drawn + for another list, or with a selection that is none, change and delete nothing.""" + title = 'Select jobs to disable or delete (the rest are enabled)' lines, rows, data = self.press(f'm|edit|0|{self.crc()}') - self.assertEqual((lines[0], rows), ('Tap a job to enable or disable it', [['1 rating', - '2 movers close US', '3 trades'], ['Select all', 'Cancel']])) + self.assertEqual((lines[0], rows), (title, [['1 rating', '2 movers close US', '3 trades'], + ['Select all', 'Cancel']])) lines, rows, data = self.press(data['1 rating']) - self.assertEqual((lines[0], rows), ('Disabled 1. rating', [['1 rating', '2 movers close US', '3 trades'], - ['Select all', 'Delete', 'OK', 'Cancel']])) - self.assertEqual(state.load(jobs.JOBS_FILE)[CHAT]['off'], ['rating']) + self.assertEqual((lines[0], rows[-1]), (title, ['Select all', 'Delete', 'OK', 'Cancel'])) + self.assertEqual(state.load(jobs.JOBS_FILE)[CHAT].get('off', []), []) # nothing is off yet + lines, rows, data = self.press(data['1 rating']) # selected again: as the jobs are + self.assertEqual(rows[-1], ['Select all', 'Cancel']) lines, rows, data = self.press(data['Select all']) - self.assertEqual((lines[0], rows[-1]), ('Disabled 2 jobs', ['Select none', 'Delete', 'OK', 'Cancel'])) + self.assertEqual(rows[-1], ['Select none', 'Delete', 'OK', 'Cancel']) lines, rows, data = self.press(data['Select none']) - self.assertEqual((lines[0], rows[-1]), ('Enabled 3 jobs', ['Select all', 'Cancel'])) + self.assertEqual(rows[-1], ['Select all', 'Cancel']) lines, rows, data = self.press(data['1 rating']) + lines, rows, data = self.press(data['OK']) + self.assertEqual((lines[0], rows[0], state.load(jobs.JOBS_FILE)[CHAT]['off']), + ('Disabled 1 job', ['+', 'Edit', 'Run...', 'Upcoming'], ['rating'])) + lines, rows, data = self.press(data['Edit']) + self.assertEqual(rows[-1], ['Select all', 'Delete', 'Cancel']) # the selection is the jobs that are off + lines, rows, data = self.press(data['2 movers close US']) + lines, rows, data = self.press(data['1 rating']) # no longer selected: it is enabled by OK + lines, rows, data = self.press(data['OK']) + self.assertEqual((lines[0], [job.on for job in self.s.chat_jobs(CHAT)]), + ('Disabled 1 job, enabled 1 job', [True, False, True])) + lines, rows, data = self.press(data['Edit']) lines, rows, data = self.press(data['Delete']) - self.assertEqual((lines[0], rows), ('Delete the 1 disabled job?', [['Yes, delete them', 'Cancel']])) - self.assertEqual(lines[2], '1. ❌ rating') + self.assertEqual((lines[0], rows), ('Delete the 1 selected job?', [['Yes, delete them', 'Cancel']])) + self.assertEqual(lines[2], '2. movers close US') lines, rows, data = self.press(data['Cancel']) - self.assertEqual(lines[0], 'Tap a job to enable or disable it') - lines, rows, data = self.press(data['OK']) - self.assertEqual((lines[0], rows[0]), ('Jobs here', ['+', 'Edit', 'Run...', 'Upcoming'])) - self.press(f'm|edit|0|{self.crc()}') - self.assertEqual(self.press('m|edit|0|ask|0000')[0][0], 'The jobs have changed: here they are now') - lines, rows, data = self.press(f'm|edit|0|{self.crc()}') + self.assertEqual(lines[0], title) lines, rows, data = self.press(data['Delete']) lines, rows, data = self.press(data['Yes, delete them']) - self.assertEqual((lines[0], [job.name for job in self.s.chat_jobs(CHAT)]), ('Deleted 1 disabled job', - ['movers close US', 'trades'])) + self.assertEqual((lines[0], [job.name for job in self.s.chat_jobs(CHAT)]), ('Deleted 1 job', + ['rating', 'trades'])) + self.assertEqual(self.press('m|edit|0|ask|1|0000')[0][0], 'The jobs have changed: here they are now') def test_the_top_level_offers_only_what_can_be_done(self): """Edit, Run and Upcoming show only where a job is there to act on: with every job off no Upcoming; with none, @@ -923,7 +931,8 @@ def test_a_stale_or_forged_press_acts_on_nothing(self): button of the older lists, j| and e|, and of the retired Enable, Disable and Delete screens among them) switches, deletes and runs nothing: the screen is drawn as the list is now, or the press is ignored.""" good = self.crc() - for data in (f'm|edit|0|1|0000', f'm|run|0|3|0000', f'm|run|0|9|{good}', f'm|run|0|0|{good}', + for data in (f'm|edit|0|m|1|0000', f'm|edit|0|ok|fff|{good}', f'm|edit|0|ok|xyz|{good}', f'm|run|0|3|0000', + f'm|run|0|9|{good}', f'm|run|0|0|{good}', f'm|run|0|x|{good}', f'm|run|0|all|{good}'): with self.subTest(data=data): lines, _, _ = self.press(data) @@ -1130,7 +1139,7 @@ def fits(service, lines, markup): lines, markup = about.add_screen(service, CHAT, steps) fits(service, about.listed_with(service, CHAT, lines), markup) fits(service, *about.next_screen(service, CHAT)) - fits(service, *about.delete_disabled_screen(service, CHAT)) + fits(service, *about.delete_screen(service, CHAT, 5)) fits(service, *about.added_screen(service, CHAT, 1)) fits(service, *about.add_failed(service, CHAT, ['5', 'd', '9'], 'that job is here already')) pages = len({about.jobs_screen(DISCORD, CHAT, 'edit', page)[0][0] for page in range(6)}) From 668921a153c2bcef4eb10acf1f0f63db9422a804 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 00:21:54 +0000 Subject: [PATCH 16/25] jobs > Edit: OK and Delete change a chat's jobs in one change, so the log says 'updated' once Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- lib/jobs.py | 49 ++++++++++++++++++++++++++--------------- lib/worker.py | 13 +++++------ tests/test_scheduler.py | 13 +++++++++++ 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/lib/jobs.py b/lib/jobs.py index 6b01259..ea2e378 100644 --- a/lib/jobs.py +++ b/lib/jobs.py @@ -588,29 +588,42 @@ def append(record): self.change(chat, append) return self.number(chat, job.key) - def delete(self, job): - """Remove a chat's job (from job() or pick()): one it added, or a default one, which stays out of its list - until the same job is added again.""" + def delete(self, *chosen): + """Remove jobs of one chat (from job() or pick()), in one change: one it added, or a default one, which stays + out of its list until the same job is added again.""" def remove(record): - """Remove the job, as stored.""" - if not job.item: - return record['jobs'].pop(self.index(record, job)) - for name in ('off', 'on'): - keep(record, name, [key for key in record.get(name, []) if key != job.item]) - record['deleted'] = list(dict.fromkeys([*removed(record), job.item])) - self.change(job.chat, remove) + """Remove the jobs, as stored.""" + for job in chosen: + if not job.item: + record['jobs'].pop(self.index(record, job)) + continue + for name in ('off', 'on'): + keep(record, name, [key for key in record.get(name, []) if key != job.item]) + record['deleted'] = list(dict.fromkeys([*removed(record), job.item])) + if chosen: + self.change(chosen[0].chat, remove) def toggle(self, job, on=None): """Switch a chat's job on (on True) or off (False), or the other way from how it is stored (None), as stored, under the state file's lock: two presses at once flip it twice. Whether it is on now.""" - def turn(record): - """Set or flip the job's switch, as stored.""" - if job.item: - return switch_item(record, job.item, not item_on(record, job.item) if on is None else on) - entry = record['jobs'][self.index(record, job)] - entry['on'] = not entry.get('on', True) if on is None else on - return entry['on'] - return self.change(job.chat, turn) + return self.change(job.chat, lambda record: self.turn(record, job, on)) + + def switch_all(self, chat, states): + """Switch several of a chat's jobs, in one change: states, (job, on) pairs.""" + def turn_all(record): + """Set each job's switch, as stored.""" + for job, on in states: + self.turn(record, job, on) + if states: + self.change(chat, turn_all) + + def turn(self, record, job, on): + """Set (on True or False) or flip (None) a job's switch in its chat's record: whether it is on now.""" + if job.item: + return switch_item(record, job.item, not item_on(record, job.item) if on is None else on) + entry = record['jobs'][self.index(record, job)] + entry['on'] = not entry.get('on', True) if on is None else on + return entry['on'] @staticmethod def index(record, job): diff --git a/lib/worker.py b/lib/worker.py index e46c607..f55af47 100755 --- a/lib/worker.py +++ b/lib/worker.py @@ -160,20 +160,17 @@ def edit_step(service, chat, scheduler, pressed, page): if what == 'ask' and chosen: return about.delete_screen(service, chat, selected, page) if what == 'ok': - off = on = 0 - try: # each set as stored, under the scheduler's lock - for i, job in enumerate(own): - if bool(selected >> i & 1) == job.on: - scheduler.toggle(job, on=not selected >> i & 1) - off, on = off + bool(selected >> i & 1), on + (not selected >> i & 1) + states = [(job, not selected >> i & 1) for i, job in enumerate(own) if bool(selected >> i & 1) == job.on] + off, on = sum(not state for _, state in states), sum(state for _, state in states) + try: # all set in one change, as stored, under the state file's lock + scheduler.switch_all(chat, states) except jobs.ScheduleError as e: return about.jobs_screen(service, chat, 'edit', page, service.escape(str(e))) said = [f"{how} {count} job{'' if count == 1 else 's'}" for count, how in ((off, 'disabled'), (on, 'enabled')) if count] return about.jobs_screen(service, chat, 'top', 0, ', '.join(said).capitalize() if said else 'Nothing changed') if what == 'yes': - for job in chosen: - scheduler.delete(job) + scheduler.delete(*chosen) s = '' if len(chosen) == 1 else 's' return about.jobs_screen(service, chat, 'top', 0, f"Deleted {len(chosen)} job{s}") return about.jobs_screen(service, chat, 'edit', page, selected=selected) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 5aaf13c..5bdef7d 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -894,6 +894,19 @@ def test_edit_selects_jobs_and_changes_nothing_until_ok(self): ['rating', 'trades'])) self.assertEqual(self.press('m|edit|0|ask|1|0000')[0][0], 'The jobs have changed: here they are now') + def test_only_a_change_is_logged_once_and_a_selection_press_not_at_all(self): + """Selecting jobs in Edit logs nothing about the schedule; OK, which changes several jobs, logs one line.""" + lines, rows, data = self.press(f'm|edit|0|{self.crc()}') + with contextlib.redirect_stderr(io.StringIO()) as log: + self.press_button(data['1 rating']) + self.assertEqual([line for line in log.getvalue().splitlines() if 'schedule' in line], []) + lines, rows, data = self.press(data['1 rating']) + lines, rows, data = self.press(data['Select all']) + with contextlib.redirect_stderr(io.StringIO()) as log: + self.press_button(data['OK']) + self.assertEqual([line for line in log.getvalue().splitlines() if 'schedule' in line], + [f'schedule: {CHAT}: updated']) + def test_the_top_level_offers_only_what_can_be_done(self): """Edit, Run and Upcoming show only where a job is there to act on: with every job off no Upcoming; with none, only +.""" From 05dce2c004d0d87466b388b235778182a5ea4ea1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 02:08:48 +0000 Subject: [PATCH 17/25] Startup logs the enabled networks and how each reaches finbot (push or pull) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- bot.py | 9 +++++++++ tests/test_pull.py | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/bot.py b/bot.py index 83022de..506aa07 100755 --- a/bot.py +++ b/bot.py @@ -55,6 +55,14 @@ def listeners(): app-level token): finbot fetches the updates of those networks itself.""" return [kind() for kind in LISTENERS if kind.wanted()] +def networks_line(): + """'networks: discord (push), slack (pull)': the networks finbot.env enables, by name, each with how its updates + reach finbot: push (it posts them to a webhook URL) or pull (finbot fetches them); 'networks: none' for none.""" + how = sorted((hook.service, 'push') if hook.path() else (hook.service, 'pull') + for hook, kind in zip(WEBHOOKS, LISTENERS) if getattr(config.env, kind.TOKENS[0]) and (hook.path() or + kind.wanted())) + return 'networks: ' + (', '.join(f'{service} ({way})' for service, way in how) or 'none') + def doubled(message): """Whether this message repeats one that the same person sent in the same chat in the last DOUBLED seconds: Telegram's mobile app can send a command twice (/start in a new chat), each its own update, and one answer is @@ -271,6 +279,7 @@ def http_server(): except config.ConfigError as e: config.print_errors(e) sys.exit(1) + log.info(networks_line()) for line in config.startup_report(): log.info('config:', line) for line in config.startup_warnings(): diff --git a/tests/test_pull.py b/tests/test_pull.py index fbc7a49..956f548 100644 --- a/tests/test_pull.py +++ b/tests/test_pull.py @@ -135,6 +135,16 @@ def test_a_listener_for_each_network_without_a_webhook(self): self.env(telegramBotToken='', telegramOutgoingWebhook='') self.assertEqual(bot.listeners(), []) + def test_the_networks_line_names_each_enabled_network_and_how_it_reaches_finbot(self): + """Startup logs the enabled networks, each pushed (its webhook URL is set) or pulled (finbot fetches).""" + self.env(telegramBotToken='1:a', telegramOutgoingWebhook='', slackBotToken='xoxb-1', slackAppToken='xapp-1', + slackOutgoingWebhook='', discordBotToken='d', discordOutgoingWebhook='https://example.com/discord') + self.assertEqual(bot.networks_line(), 'networks: discord (push), slack (pull), telegram (pull)') + self.env(slackAppToken='', discordBotToken='', telegramOutgoingWebhook='https://example.com/t') + self.assertEqual(bot.networks_line(), 'networks: telegram (push)') # Slack has no way to hear; Discord is off + self.env(telegramBotToken='') + self.assertEqual(bot.networks_line(), 'networks: none') + def test_a_press_polled_for_is_the_press_the_webhook_gets(self): """The same button press, posted to the webhook (http mode) or polled for (pull mode), reaches the worker as the same Press, and is acknowledged the same way.""" From 30a07eba26f295295593a319f820c05fbd51a723 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 06:42:09 +0000 Subject: [PATCH 18/25] Discord: a refused interaction's token stays out of the log; the signature warning explains Discord's deliberate bad probe Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- lib/discord_webhook.py | 6 ++++-- tests/test_discord.py | 9 +++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/discord_webhook.py b/lib/discord_webhook.py index 718811b..91d2038 100644 --- a/lib/discord_webhook.py +++ b/lib/discord_webhook.py @@ -11,6 +11,7 @@ class DiscordInteractions(inbound.Webhook): the app's Ed25519 key, whose public half is discordPublicKey.""" URL = 'discordOutgoingWebhook' REFUSED = inbound.UNAUTHORIZED # Discord checks that a bad signature gets a 401, and drops a URL answering otherwise + SECRET_FIELDS = ('token',) # the interaction's token, with which anyone can answer it for 15 minutes def refuse(self, environ, body, update): """The request must carry Discord's signature of its timestamp and body under discordPublicKey, checked with @@ -22,10 +23,11 @@ def refuse(self, environ, body, update): from nacl.exceptions import BadSignatureError from nacl.signing import VerifyKey try: - VerifyKey(bytes.fromhex(config.env.discordPublicKey)).verify(timestamp.encode() + body, + VerifyKey(bytes.fromhex(config.env.discordPublicKey.strip())).verify(timestamp.encode() + body, bytes.fromhex(signature)) except (BadSignatureError, ValueError): - return "Discord signature incorrect" + return ("Discord signature incorrect (Discord sends one on purpose when the Interactions Endpoint URL is " + "saved, to see a 401; if saving fails, discordPublicKey is not General Information > Public Key)") return None diff --git a/tests/test_discord.py b/tests/test_discord.py index b4eb312..ed726ed 100644 --- a/tests/test_discord.py +++ b/tests/test_discord.py @@ -120,6 +120,15 @@ def post(self, update, signature=None, timestamp='1700000000'): HTTP_X_SIGNATURE_TIMESTAMP=timestamp), lambda s, *a: status.append(s)) return status[0], b''.join(out), out + def test_a_refused_interaction_keeps_its_token_out_of_the_log(self): + """An interaction whose signature fails is logged without its token, which answers it for 15 minutes.""" + body = json.dumps(interaction(1, token='secret-token')).encode() + with contextlib.redirect_stderr(io.StringIO()) as log: + bot.main(wsgi_environ('/discord', body, HTTP_X_SIGNATURE_ED25519='00' * 64, + HTTP_X_SIGNATURE_TIMESTAMP='1700000000'), lambda *a: None) + self.assertEqual(('secret-token' in log.getvalue(), 'Discord signature incorrect' in log.getvalue()), + (False, True)) + def test_a_signed_check_gets_its_pong(self): """Discord's check of the URL, signed, gets its PONG.""" status, answer, _ = self.post(interaction(1)) From 8603f6e0c74b67c0849e7b6e64d10ca72b1293fd Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 06:48:11 +0000 Subject: [PATCH 19/25] Discord: log the endpoint check that passes; a refused update is one warning line, its body at debug Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- bot.py | 4 ++-- lib/discord_common.py | 1 + lib/discord_webhook.py | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/bot.py b/bot.py index 506aa07..1be519e 100755 --- a/bot.py +++ b/bot.py @@ -131,8 +131,8 @@ def print_headers(say=log.info): return respond(inbound.BAD_REQUEST) if why := source.refuse(environ, body, update): shown = {k: '' if k in source.SECRET_FIELDS else v for k, v in update.items()} - log.warn("warning:", why) - log.warn(f"{source.service.capitalize()} refused update", json.dumps(shown, indent=4, default=str)) + log.warn(f"{source.service.capitalize()} refused update: {why}") + log.debug(f"{source.service.capitalize()} refused update", json.dumps(shown, indent=4, default=str)) return respond(source.REFUSED) environ[AUTHENTICATED] = True return respond(take(source.network, update)) diff --git a/lib/discord_common.py b/lib/discord_common.py index bce4486..035c244 100644 --- a/lib/discord_common.py +++ b/lib/discord_common.py @@ -130,6 +130,7 @@ def interaction(self, update): UNSUPPORTED.""" kind = update.get('type') if kind == self.PING: + log.info("Discord: endpoint check (a signed PING) answered") return inbound.Response(body=json.dumps({'type': self.PONG}).encode(), interaction=True) person = (update.get('member') or {}).get('user') or update.get('user') or {} channel = str(update.get('channel_id') or (update.get('channel') or {}).get('id') or '') diff --git a/lib/discord_webhook.py b/lib/discord_webhook.py index 91d2038..12f2775 100644 --- a/lib/discord_webhook.py +++ b/lib/discord_webhook.py @@ -26,8 +26,9 @@ def refuse(self, environ, body, update): VerifyKey(bytes.fromhex(config.env.discordPublicKey.strip())).verify(timestamp.encode() + body, bytes.fromhex(signature)) except (BadSignatureError, ValueError): - return ("Discord signature incorrect (Discord sends one on purpose when the Interactions Endpoint URL is " - "saved, to see a 401; if saving fails, discordPublicKey is not General Information > Public Key)") + return ("Discord signature incorrect (expected once or twice when the Interactions Endpoint URL is saved: " + "Discord sends a bad one on purpose to see a 401; if saving fails, discordPublicKey is not General " + "Information > Public Key)") return None From 85920e410d48b705e3904e4edf68d855f7c510ea Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 23:56:59 +0000 Subject: [PATCH 20/25] Keyboard: .pe and .jobs buttons, sorted; /peg is a slash command of its own Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- CLAUDE.md | 2 +- doc/commands.md | 2 +- lib/worker.py | 14 +++++++++----- tests/test_reports.py | 14 +++++++++++--- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 68386a6..d249017 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,7 +75,7 @@ An open PR takes in every commit pushed to its branch, so a PR opened for one sm failure in a row: `backoff`; `SocketListener`, a connection a round, and `WebSocket`, over websocket-client, for Slack and Discord), `lib/transports.py` (the `Transport` base, below). - `lib/worker.py`: `COMMANDS`, a table of `Command(names, handler, usage, section, buttons, percent, period, - keywords, words, takes, job)`, which also makes `.help`, the commands a job may run (`job_commands()`, each a + keywords, words, takes, job, slashes)`, which also makes `.help`, the commands a job may run (`job_commands()`, each a `CommandJob`: every one but those that change something, `job=False`, and those a report runs, `REPORTED`; stored under its `key`, `.price`'s as compare, `STORED`), the Telegram command keyboard (`KEYBOARD`, which `telegram_common.keep_keyboard` gives each DM after a command where it has another or none, and after /start and diff --git a/doc/commands.md b/doc/commands.md index 84f3a8e..855848c 100644 --- a/doc/commands.md +++ b/doc/commands.md @@ -259,7 +259,7 @@ Command: ### ratio (pe, peg) ![Forward PE report in Slack](../img/finbot_fpe.png?raw=true "Forward PE report in Slack") -List the stocks followed by a ratio: `/ratio` is the forward P/E, lowest first; `/ratio current` the trailing P/E (`current` or `trailing`), of the earnings reported; `/ratio peg` the PEG. Then, optionally, the end: `top` (the default), `bottom` or `reverse` for the highest first, or `negative`, in either order: `/ratio peg reverse`, `/ratio current negative`. A ticker (`/ratio T1`) gets its own figures. `/pe` and `/peg` still work, as `/ratio` and `/ratio peg`. A forward P/E and a PEG are analysts' estimates (of earnings, of their growth), so their lists take only stocks covered by at least `min_analysts` analysts (7 by default; see `settings.conf`), as `/buy` and `/sell` do; a trailing P/E is no estimate, so every stock counts. +List the stocks followed by a ratio: `/ratio` is the forward P/E, lowest first; `/ratio current` the trailing P/E (`current` or `trailing`), of the earnings reported; `/ratio peg` the PEG. Then, optionally, the end: `top` (the default), `bottom` or `reverse` for the highest first, or `negative`, in either order: `/ratio peg reverse`, `/ratio current negative`. A ticker (`/ratio T1`) gets its own figures. `/pe` is `/ratio` and `/peg` is `/ratio peg` (both typed, `/peg` also in the slash menu; Telegram's keyboard has `.pe`). A forward P/E and a PEG are analysts' estimates (of earnings, of their growth), so their lists take only stocks covered by at least `min_analysts` analysts (7 by default; see `settings.conf`), as `/buy` and `/sell` do; a trailing P/E is no estimate, so every stock counts. Command: ``` diff --git a/lib/worker.py b/lib/worker.py index f55af47..b6a3022 100755 --- a/lib/worker.py +++ b/lib/worker.py @@ -691,6 +691,7 @@ class Command: words: tuple = (0, None) # how many words it takes, at least and at most (None: any): others get its usage takes: str = '' # what its words are, for suggest and a job's example: 'ticker' or 'portfolio' (a name) job: bool = True # whether a job may run it (job_commands): not one that changes something + slashes: dict = dataclasses.field(default_factory=dict) # other names that are slash commands too: {name: arguments} def fits(self, args): """Whether it takes as many words as args has (words).""" @@ -743,15 +744,16 @@ def fits(self, args): keywords=TOP | {name: name for name in SESSIONS}, words=(0, 1)), # a word: session's or premarket's SYMBOL Command(BUY_SELL, on_rating, ('.buysell',), RISK, ('.buy & sell',)), # .buy & sell, .buy sell: both Command(('ratio', 'pe', 'forward pe', 'forwardpe', 'fpe', 'peg'), on_ratio, - ('.ratio [SYMBOL] [forward|current|peg] [top|bottom|negative|reverse]',), RISK, ('.ratio',), keywords=ENDS, - options=RATIOS, words=(0, 1), takes='ticker'), # was .pe (the forward P/E) and .peg, still their names + ('.ratio [SYMBOL] [forward|current|peg] [top|bottom|negative|reverse]',), RISK, ('.pe',), keywords=ENDS, + options=RATIOS, words=(0, 1), takes='ticker', slashes={'peg': '[SYMBOL] [top|bottom|negative|reverse]'}), + # was .pe (the forward P/E) and .peg, still their names, /peg a slash command of its own Command(('shorts', 'short'), on_shorts, ('.shorts [percent|SYMBOL]',), RISK, ('.shorts',), percent=True, words=(0, 1), takes='ticker'), Command(('trades', 'trade'), on_trades, ('.trades [period] [portfolio]',), TRADES, ('.trades',), period=True, takes='portfolio'), Command(('jobs', 'job', 'cron', 'crons', 'schedule'), on_jobs, ('.jobs [upcoming|system]', '.jobs enable|disable|delete|run [NUMBER]', '.jobs add REPORT SCHEDULE', '.jobs add message SCHEDULE "TEXT"'), - JOBS, ('.schedule',), keywords=JOB_CHANGES, job=False), + JOBS, ('.jobs',), keywords=JOB_CHANGES, job=False), PROFILE, ] KEYBOARD = tuple(text for command in COMMANDS for text in command.buttons) # Telegram's command keyboard in a DM @@ -797,8 +799,8 @@ class Slash(typing.NamedTuple): def slash_commands(): """The slash commands to register (Slash), from the .help lines (slash_form), and /help; the arguments of a - command's several lines joined by " | ". A command's other names (/profile, /job) are no slash commands: typed, - they work as every name does.""" + command's several lines joined by " | ". A command's other names (/profile, /job) are no slash commands, but + those it lists in slashes (/peg): typed, they work as every name does.""" found = {'help': (None, [])} # name -> (its command, the arguments of its lines) for command in COMMANDS: for line in command.usage: @@ -806,6 +808,8 @@ def slash_commands(): name, arguments = form if arguments not in found.setdefault(name, (command, []))[1]: found[name][1].append(arguments) + for name, arguments in command.slashes.items(): + found.setdefault(name, (command, []))[1].append(arguments) return [Slash('help', 'every command, and what it takes', '') if name == 'help' else Slash(name, described(name, lines), ' | '.join(lines), completes(command)) for name, (command, lines) in found.items()] diff --git a/tests/test_reports.py b/tests/test_reports.py index f617195..0a5c182 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -115,17 +115,25 @@ def test_buy_and_sell_share_one_chart_with_the_sells_red(self): self.assertNotIn('No tracked stocks rated buy by 7+ analysts', payload) def test_the_keyboard_is_alphabetical_and_every_button_is_its_command(self): - """The command keyboard is alphabetical, .schedule (the jobs) in place of .beta, and each button runs the + """The command keyboard is alphabetical, with .pe (the ratios) and .jobs, and each button runs the command it belongs to in the table, not taken for a ticker by .SYMBOL.""" buttons = [button['text'] for row in telegram_common.keyboard(worker.KEYBOARD)['keyboard'] for button in row] self.assertEqual(buttons, sorted(buttons)) - self.assertEqual(buttons, ['.buy & sell', '.earnings', '.movers', '.performance', - '.price', '.ratio', '.schedule', '.shorts', '.trades', '.treemap', '.watchlist']) + self.assertEqual(buttons, ['.buy & sell', '.earnings', '.jobs', '.movers', '.pe', '.performance', + '.price', '.shorts', '.trades', '.treemap', '.watchlist']) for command in worker.COMMANDS: for text in command.buttons: with self.subTest(text=text): self.assertIs(worker.read(text, BOT)[0], command) + def test_peg_is_a_slash_command_that_answers_as_ratio_peg(self): + """/peg is in the slash menu beside /ratio, with the ending words /ratio takes, and answers as .ratio peg.""" + slashes = {slash.name: slash for slash in worker.slash_commands()} + self.assertEqual(slashes['peg'].arguments, '[SYMBOL] [top|bottom|negative|reverse]') + self.assertTrue(slashes['peg'].completes) + command, args = worker.read('/peg reverse', BOT) + self.assertEqual((command.handler, args.word, args.keyword), (worker.on_ratio, 'peg', 'bottom')) + def test_history_and_compare(self): """.history and .compare answer.""" for cmd in ('.history T1', '.compare T1 T2', '.compare T1 T2 3m'): From d85ad4ee0826675e7977750f063881ada5eda2e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 23:58:28 +0000 Subject: [PATCH 21/25] No /peg slash command: .peg and /peg are typed names of /ratio peg, as before Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- CLAUDE.md | 2 +- doc/commands.md | 2 +- lib/worker.py | 11 ++++------- tests/test_reports.py | 8 +++----- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d249017..68386a6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,7 +75,7 @@ An open PR takes in every commit pushed to its branch, so a PR opened for one sm failure in a row: `backoff`; `SocketListener`, a connection a round, and `WebSocket`, over websocket-client, for Slack and Discord), `lib/transports.py` (the `Transport` base, below). - `lib/worker.py`: `COMMANDS`, a table of `Command(names, handler, usage, section, buttons, percent, period, - keywords, words, takes, job, slashes)`, which also makes `.help`, the commands a job may run (`job_commands()`, each a + keywords, words, takes, job)`, which also makes `.help`, the commands a job may run (`job_commands()`, each a `CommandJob`: every one but those that change something, `job=False`, and those a report runs, `REPORTED`; stored under its `key`, `.price`'s as compare, `STORED`), the Telegram command keyboard (`KEYBOARD`, which `telegram_common.keep_keyboard` gives each DM after a command where it has another or none, and after /start and diff --git a/doc/commands.md b/doc/commands.md index 855848c..5970ed0 100644 --- a/doc/commands.md +++ b/doc/commands.md @@ -259,7 +259,7 @@ Command: ### ratio (pe, peg) ![Forward PE report in Slack](../img/finbot_fpe.png?raw=true "Forward PE report in Slack") -List the stocks followed by a ratio: `/ratio` is the forward P/E, lowest first; `/ratio current` the trailing P/E (`current` or `trailing`), of the earnings reported; `/ratio peg` the PEG. Then, optionally, the end: `top` (the default), `bottom` or `reverse` for the highest first, or `negative`, in either order: `/ratio peg reverse`, `/ratio current negative`. A ticker (`/ratio T1`) gets its own figures. `/pe` is `/ratio` and `/peg` is `/ratio peg` (both typed, `/peg` also in the slash menu; Telegram's keyboard has `.pe`). A forward P/E and a PEG are analysts' estimates (of earnings, of their growth), so their lists take only stocks covered by at least `min_analysts` analysts (7 by default; see `settings.conf`), as `/buy` and `/sell` do; a trailing P/E is no estimate, so every stock counts. +List the stocks followed by a ratio: `/ratio` is the forward P/E, lowest first; `/ratio current` the trailing P/E (`current` or `trailing`), of the earnings reported; `/ratio peg` the PEG. Then, optionally, the end: `top` (the default), `bottom` or `reverse` for the highest first, or `negative`, in either order: `/ratio peg reverse`, `/ratio current negative`. A ticker (`/ratio T1`) gets its own figures. `/pe` and `/peg` still work when typed, as `/ratio` and `/ratio peg` (only `/ratio` is in the slash menu); Telegram's keyboard has `.pe`. A forward P/E and a PEG are analysts' estimates (of earnings, of their growth), so their lists take only stocks covered by at least `min_analysts` analysts (7 by default; see `settings.conf`), as `/buy` and `/sell` do; a trailing P/E is no estimate, so every stock counts. Command: ``` diff --git a/lib/worker.py b/lib/worker.py index b6a3022..dd36785 100755 --- a/lib/worker.py +++ b/lib/worker.py @@ -691,7 +691,6 @@ class Command: words: tuple = (0, None) # how many words it takes, at least and at most (None: any): others get its usage takes: str = '' # what its words are, for suggest and a job's example: 'ticker' or 'portfolio' (a name) job: bool = True # whether a job may run it (job_commands): not one that changes something - slashes: dict = dataclasses.field(default_factory=dict) # other names that are slash commands too: {name: arguments} def fits(self, args): """Whether it takes as many words as args has (words).""" @@ -745,8 +744,8 @@ def fits(self, args): Command(BUY_SELL, on_rating, ('.buysell',), RISK, ('.buy & sell',)), # .buy & sell, .buy sell: both Command(('ratio', 'pe', 'forward pe', 'forwardpe', 'fpe', 'peg'), on_ratio, ('.ratio [SYMBOL] [forward|current|peg] [top|bottom|negative|reverse]',), RISK, ('.pe',), keywords=ENDS, - options=RATIOS, words=(0, 1), takes='ticker', slashes={'peg': '[SYMBOL] [top|bottom|negative|reverse]'}), - # was .pe (the forward P/E) and .peg, still their names, /peg a slash command of its own + options=RATIOS, words=(0, 1), takes='ticker'), + # was .pe (the forward P/E) and .peg, still their names Command(('shorts', 'short'), on_shorts, ('.shorts [percent|SYMBOL]',), RISK, ('.shorts',), percent=True, words=(0, 1), takes='ticker'), Command(('trades', 'trade'), on_trades, ('.trades [period] [portfolio]',), TRADES, ('.trades',), period=True, @@ -799,8 +798,8 @@ class Slash(typing.NamedTuple): def slash_commands(): """The slash commands to register (Slash), from the .help lines (slash_form), and /help; the arguments of a - command's several lines joined by " | ". A command's other names (/profile, /job) are no slash commands, but - those it lists in slashes (/peg): typed, they work as every name does.""" + command's several lines joined by " | ". A command's other names (/profile, /job) are no slash commands: typed, + they work as every name does.""" found = {'help': (None, [])} # name -> (its command, the arguments of its lines) for command in COMMANDS: for line in command.usage: @@ -808,8 +807,6 @@ def slash_commands(): name, arguments = form if arguments not in found.setdefault(name, (command, []))[1]: found[name][1].append(arguments) - for name, arguments in command.slashes.items(): - found.setdefault(name, (command, []))[1].append(arguments) return [Slash('help', 'every command, and what it takes', '') if name == 'help' else Slash(name, described(name, lines), ' | '.join(lines), completes(command)) for name, (command, lines) in found.items()] diff --git a/tests/test_reports.py b/tests/test_reports.py index 0a5c182..38e6504 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -126,11 +126,9 @@ def test_the_keyboard_is_alphabetical_and_every_button_is_its_command(self): with self.subTest(text=text): self.assertIs(worker.read(text, BOT)[0], command) - def test_peg_is_a_slash_command_that_answers_as_ratio_peg(self): - """/peg is in the slash menu beside /ratio, with the ending words /ratio takes, and answers as .ratio peg.""" - slashes = {slash.name: slash for slash in worker.slash_commands()} - self.assertEqual(slashes['peg'].arguments, '[SYMBOL] [top|bottom|negative|reverse]') - self.assertTrue(slashes['peg'].completes) + def test_peg_is_typed_as_ratio_peg_and_has_no_slash_command_of_its_own(self): + """Only /ratio is in the slash menu; .peg and /peg typed answer as .ratio peg.""" + self.assertNotIn('peg', {slash.name for slash in worker.slash_commands()}) command, args = worker.read('/peg reverse', BOT) self.assertEqual((command.handler, args.word, args.keyword), (worker.on_ratio, 'peg', 'bottom')) From eec42a3d2fc5dd0bd6021a21d4cb1dd6c5d52fd5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 00:00:51 +0000 Subject: [PATCH 22/25] Discord: an interaction's webhook answering Unknown Webhook (10015) is tried again after short waits Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- lib/discord_common.py | 23 +++++++++++++++++++++-- tests/test_discord.py | 21 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/lib/discord_common.py b/lib/discord_common.py index 035c244..409d16a 100644 --- a/lib/discord_common.py +++ b/lib/discord_common.py @@ -4,6 +4,7 @@ that posts into its channels and DMs (DiscordBot).""" import json import re +import time import requests @@ -21,6 +22,7 @@ NO_MENTIONS = {'parse': []} # text that looks like @everyone or a mention pings nobody GUILD, BOT_DM = 0, 1 # where a slash command may be used: in servers, and in a DM with the bot OPTION = 'arguments' # the name of a slash command's one option: its arguments as text +NOT_YET_WAITS = (0.5, 1, 2) # seconds between tries of a call Discord answered 'Unknown Webhook' (see NotYet) USER_AGENT = 'DiscordBot (https://github.com/robdevops/finbot, {})' # the form Discord asks of every request's agent @@ -30,7 +32,22 @@ def call(method, path, **kwargs): reason when it refuses, or a requests error.""" headers = {'Authorization': 'Bot ' + config.env.discordBotToken, 'User-Agent': USER_AGENT.format(config.git_version()), **kwargs.pop('headers', {})} - return net.patiently(lambda: answer_of(net.request(method, API + path, headers=headers, **kwargs), method, path)) + def send(): + """The request, a rate limit waited out once.""" + return net.patiently( + lambda: answer_of(net.request(method, API + path, headers=headers, **kwargs), method, path)) + for wait in NOT_YET_WAITS: + try: + return send() + except NotYet as e: + log.warn(f"{net.redact(e)}: Discord has not registered the interaction yet, trying again in {wait:g} s") + time.sleep(wait) + return send() + + +class NotYet(RuntimeError): + """Discord's answer 'Unknown Webhook' (10015) to a call on an interaction's webhook: Discord may not have taken the + answer to the interaction yet, which it must have before the interaction's webhook is known.""" def answer_of(response, method, path): @@ -38,7 +55,9 @@ def answer_of(response, method, path): RuntimeError with Discord's reason for another refusal.""" if response.status_code >= 400: said = f"Discord {method} {path}: {response.status_code} {reason(response)}" - raise net.RateLimited(said, net.retry_after(response)) if response.status_code == 429 else RuntimeError(said) + if response.status_code == 429: + raise net.RateLimited(said, net.retry_after(response)) + raise (NotYet if path.startswith('/webhooks/') and said.endswith('(10015)') else RuntimeError)(said) try: return response.json() except ValueError: diff --git a/tests/test_discord.py b/tests/test_discord.py index ed726ed..667f451 100644 --- a/tests/test_discord.py +++ b/tests/test_discord.py @@ -263,6 +263,27 @@ def test_a_command_in_a_message_shows_typing(self): self.assertEqual(self.calls[0][:2], ('POST', '/channels/77/typing')) self.assertLess(chat.TYPING[1], 10) + def test_an_interaction_discord_has_not_registered_yet_is_tried_again(self): + """An answer 'Unknown Webhook' (10015) on an interaction's webhook, as when the edit follows the interaction's + answer too closely, is tried again after short waits, and raises when it persists; other 404s raise at once.""" + slept = [] + self.patch(discord_common.time, 'sleep', slept.append) + path = '/webhooks/42/tok/messages/@original' + self.answers += [Resp({'message': 'Unknown Webhook', 'code': 10015}, 404)] * 2 + [Resp({'id': '5'})] + with contextlib.redirect_stderr(io.StringIO()): + self.assertEqual(discord_common.call('PATCH', path), {'id': '5'}) + self.assertEqual((slept, len(self.calls)), ([0.5, 1], 3)) + slept.clear() + self.answers += [Resp({'message': 'Unknown Webhook', 'code': 10015}, 404)] * 4 + with contextlib.redirect_stderr(io.StringIO()), self.assertRaisesRegex(RuntimeError, '10015'): + discord_common.call('PATCH', path) + self.assertEqual(slept, [0.5, 1, 2]) + slept.clear() + self.answers.append(Resp({'message': 'Unknown Message', 'code': 10008}, 404)) + with contextlib.redirect_stderr(io.StringIO()), self.assertRaisesRegex(RuntimeError, '10008'): + discord_common.call('PATCH', path) + self.assertEqual(slept, []) + def test_each_request_names_finbot_as_discord_asks(self): """Each request carries the bot token, and the User-Agent that Discord asks of a bot: DiscordBot, its URL and its version (finbot's commit).""" From 45f36ab1a8be030b389dde44b6ae0b5e051d2fd7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 00:08:24 +0000 Subject: [PATCH 23/25] Treemap grouping menu: Done, as its last button Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/commands.md | 2 +- lib/reports/pick.py | 4 ++-- tests/test_reports.py | 17 +++++++++-------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/doc/commands.md b/doc/commands.md index 5970ed0..f9f1655 100644 --- a/doc/commands.md +++ b/doc/commands.md @@ -65,7 +65,7 @@ The bot answers commands in every chat it is in. By default, it fetches its upda * A watch list per chat, which anyone in it can change, picked up by the reports there * Listing of portfolios and portfolio performance * A treemap of a portfolio (`/treemap portfolio [sector|industry|country|currency|market]`): the treemap tiles each holding by value, grouped and coloured by industry (the default), sector, country (its market's, as two letters, with a flag in the caption), currency or market -* Buttons to pick from where a command takes a portfolio or tickers and is typed without them: `/treemap`, `/holdings` (the first portfolio alphabetically; a treemap by industry, or by the grouping typed) and `/price` (the first ticker, over 7D unless a period or `max` is typed) answer at once, with the buttons under the chart: portfolios four to a row and `/treemap`'s Grouped by button after them, which opens a menu of Country, Currency, Industry, Market and Sector with Back (the one shown marked; a choice redoes the chart in place and keeps the menu open, Back returns to the portfolios); for tickers the period row, ending in `+` to the tickers (those held, else the chat's watchlist) with `β—€` `β–Ά` pages: up to 8 ticked (drawn in the network's selection colour), which OK draws (a ninth is refused with a message to the presser alone, the chart left as it is). With one ticker ticked, `+` clears it, so that the first tick picks afresh. Another portfolio or ticker, a grouping or a period redoes the chart in place (a tick or a page of tickers changes only the buttons, on Telegram and Discord), and a `/treemap` draws its other charts in the background so the next press is quick. `/price T1 T2 [period]` typed with tickers gets the same period row and `+` when the tickers are among those held (else watched) and the period is a button one. Every chart with a period row, from any command, draws its other periods in the background once it is sent, so a period press is quick. Thirty-two items make a page (fewer on Discord, which holds five rows of five buttons). A job of one posts the same, buttons and all (but draws no charts ahead) +* Buttons to pick from where a command takes a portfolio or tickers and is typed without them: `/treemap`, `/holdings` (the first portfolio alphabetically; a treemap by industry, or by the grouping typed) and `/price` (the first ticker, over 7D unless a period or `max` is typed) answer at once, with the buttons under the chart: portfolios four to a row and `/treemap`'s Grouped by button after them, which opens a menu of Country, Currency, Industry, Market and Sector with Done last (the one shown marked; a choice redoes the chart in place and keeps the menu open, Done returns to the portfolios); for tickers the period row, ending in `+` to the tickers (those held, else the chat's watchlist) with `β—€` `β–Ά` pages: up to 8 ticked (drawn in the network's selection colour), which OK draws (a ninth is refused with a message to the presser alone, the chart left as it is). With one ticker ticked, `+` clears it, so that the first tick picks afresh. Another portfolio or ticker, a grouping or a period redoes the chart in place (a tick or a page of tickers changes only the buttons, on Telegram and Discord), and a `/treemap` draws its other charts in the background so the next press is quick. `/price T1 T2 [period]` typed with tickers gets the same period row and `+` when the tickers are among those held (else watched) and the period is a button one. Every chart with a period row, from any command, draws its other periods in the background once it is sent, so a period press is quick. Thirty-two items make a page (fewer on Discord, which holds five rows of five buttons). A job of one posts the same, buttons and all (but draws no charts ahead) * Listing of holdings sorted by various attributes * Running the other stock reports on demand diff --git a/lib/reports/pick.py b/lib/reports/pick.py index 9bb8dda..f58bda2 100644 --- a/lib/reports/pick.py +++ b/lib/reports/pick.py @@ -95,12 +95,12 @@ def screen(service, cmd, found, group, period, shown, mask, view, page=0, note=N step = 'n' if note else 'm' # the tickers' screen, at a page button = press_button(cmd, crc, group, period, shown, mask) rows = [] - if view == 'g': # the treemap's menu, by name in rows of three: a choice redoes the chart and stays; Back + if view == 'g': # the treemap's menu, by name in rows of three: a choice redoes the chart and stays; Done rows = buttons.chunks([button(name.capitalize(), f"q{page}", name == holdings.grouping(group), group=key) for key, name in sorted(holdings.GROUPS.items(), key=lambda item: item[1])], 3) if len(rows[-1]) == 3: rows.append([]) - rows[-1].insert(0, button('Back', f"p{page}")) # first in the last row + rows[-1].append(button('Done', f"p{page}")) # last, on a row of its own where the last is full return ['Grouped by'], rows if cmd == 'c' and view == 'p': # the periods, and + to the tickers rows.append([button(buttons.PERIODS[key][0], 'p0', key == period, period=key) for key in buttons.BUTTONS['c']] + diff --git a/tests/test_reports.py b/tests/test_reports.py index 38e6504..7ee5a8e 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -1168,20 +1168,21 @@ def test_portfolios_come_four_to_a_row(self): def test_the_group_menu_and_another_portfolio_redraw_the_answer(self): """The Grouped by button opens a menu of the five groupings, the one shown marked; a choice redoes the chart in - place and keeps the menu, Back returns to the portfolio buttons; another portfolio's button redoes it too.""" + place and keeps the menu, Done (the last button) returns to the portfolio buttons; another portfolio's button + redoes it too.""" self.portfolios('Amy', 'Zed') self.command('.treemap') kind, lines, rows = self.tap('Grouped by: Industry β–Ύ') self.assertEqual((kind, self.labels(rows)), ('buttons', - [['Country', 'Currency', '● Industry'], ['Back', 'Market', 'Sector']])) + [['Country', 'Currency', '● Industry'], ['Market', 'Sector', 'Done']])) kind, lines, rows = self.tap('Sector') self.assertEqual((kind, lines[0], self.labels(rows)), ('photo', 'Amy treemap by sector', - [['Country', 'Currency', 'Industry'], ['Back', 'Market', '● Sector']])) # the menu stays + [['Country', 'Currency', 'Industry'], ['Market', '● Sector', 'Done']])) # the menu stays self.assertEqual(self.toasts[-1], ('Updating…', False)) kind, lines, rows = self.tap('Currency') self.assertEqual((kind, lines[0], self.labels(rows)), ('photo', 'Amy treemap by currency', - [['Country', '● Currency', 'Industry'], ['Back', 'Market', 'Sector']])) - kind, lines, rows = self.tap('Back') + [['Country', '● Currency', 'Industry'], ['Market', 'Sector', 'Done']])) + kind, lines, rows = self.tap('Done') self.assertEqual((kind, lines[0], self.labels(rows)), ('photo', 'Amy treemap by currency', [['● Amy', 'Zed'], ['Grouped by: Currency β–Ύ']])) kind, lines, rows = self.tap('Zed') @@ -1189,14 +1190,14 @@ def test_the_group_menu_and_another_portfolio_redraw_the_answer(self): [['Amy', '● Zed'], ['Grouped by: Currency β–Ύ']])) def test_the_group_menu_has_a_button_for_each_grouping(self): - """The Grouped by menu is made from GROUPS, by name in rows of three, Back first in the last row: a new grouping + """The Grouped by menu is made from GROUPS, by name in rows of three, Done last: a new grouping gets its button (the five were placed by hand).""" menu = lambda: [[('● ' if button.active else '') + button.label for button in row] for row in pick.screen(TELEGRAM, 't', ['Amy', 'Zed'], '0', '0', 1, 0, 'g')[1]] - self.assertEqual(menu(), [['Country', 'Currency', '● Industry'], ['Back', 'Market', 'Sector']]) + self.assertEqual(menu(), [['Country', 'Currency', '● Industry'], ['Market', 'Sector', 'Done']]) with mock.patch.dict(holdings.GROUPS, {'x': 'exchange'}): self.assertEqual(menu(), [['Country', 'Currency', 'Exchange'], ['● Industry', 'Market', 'Sector'], - ['Back']]) + ['Done']]) def test_a_holdings_answer_keeps_its_buttons(self): """.holdings: another portfolio's list as text in place of the first's, the portfolio buttons still under it.""" From b75953419024cc34687fc613ad2f1ded175cb10c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 00:09:35 +0000 Subject: [PATCH 24/25] Treemap grouping menu: the last button reads OK Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- doc/commands.md | 2 +- lib/reports/pick.py | 4 ++-- tests/test_reports.py | 16 ++++++++-------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/doc/commands.md b/doc/commands.md index f9f1655..5a065a3 100644 --- a/doc/commands.md +++ b/doc/commands.md @@ -65,7 +65,7 @@ The bot answers commands in every chat it is in. By default, it fetches its upda * A watch list per chat, which anyone in it can change, picked up by the reports there * Listing of portfolios and portfolio performance * A treemap of a portfolio (`/treemap portfolio [sector|industry|country|currency|market]`): the treemap tiles each holding by value, grouped and coloured by industry (the default), sector, country (its market's, as two letters, with a flag in the caption), currency or market -* Buttons to pick from where a command takes a portfolio or tickers and is typed without them: `/treemap`, `/holdings` (the first portfolio alphabetically; a treemap by industry, or by the grouping typed) and `/price` (the first ticker, over 7D unless a period or `max` is typed) answer at once, with the buttons under the chart: portfolios four to a row and `/treemap`'s Grouped by button after them, which opens a menu of Country, Currency, Industry, Market and Sector with Done last (the one shown marked; a choice redoes the chart in place and keeps the menu open, Done returns to the portfolios); for tickers the period row, ending in `+` to the tickers (those held, else the chat's watchlist) with `β—€` `β–Ά` pages: up to 8 ticked (drawn in the network's selection colour), which OK draws (a ninth is refused with a message to the presser alone, the chart left as it is). With one ticker ticked, `+` clears it, so that the first tick picks afresh. Another portfolio or ticker, a grouping or a period redoes the chart in place (a tick or a page of tickers changes only the buttons, on Telegram and Discord), and a `/treemap` draws its other charts in the background so the next press is quick. `/price T1 T2 [period]` typed with tickers gets the same period row and `+` when the tickers are among those held (else watched) and the period is a button one. Every chart with a period row, from any command, draws its other periods in the background once it is sent, so a period press is quick. Thirty-two items make a page (fewer on Discord, which holds five rows of five buttons). A job of one posts the same, buttons and all (but draws no charts ahead) +* Buttons to pick from where a command takes a portfolio or tickers and is typed without them: `/treemap`, `/holdings` (the first portfolio alphabetically; a treemap by industry, or by the grouping typed) and `/price` (the first ticker, over 7D unless a period or `max` is typed) answer at once, with the buttons under the chart: portfolios four to a row and `/treemap`'s Grouped by button after them, which opens a menu of Country, Currency, Industry, Market and Sector with OK last (the one shown marked; a choice redoes the chart in place and keeps the menu open, OK returns to the portfolios); for tickers the period row, ending in `+` to the tickers (those held, else the chat's watchlist) with `β—€` `β–Ά` pages: up to 8 ticked (drawn in the network's selection colour), which OK draws (a ninth is refused with a message to the presser alone, the chart left as it is). With one ticker ticked, `+` clears it, so that the first tick picks afresh. Another portfolio or ticker, a grouping or a period redoes the chart in place (a tick or a page of tickers changes only the buttons, on Telegram and Discord), and a `/treemap` draws its other charts in the background so the next press is quick. `/price T1 T2 [period]` typed with tickers gets the same period row and `+` when the tickers are among those held (else watched) and the period is a button one. Every chart with a period row, from any command, draws its other periods in the background once it is sent, so a period press is quick. Thirty-two items make a page (fewer on Discord, which holds five rows of five buttons). A job of one posts the same, buttons and all (but draws no charts ahead) * Listing of holdings sorted by various attributes * Running the other stock reports on demand diff --git a/lib/reports/pick.py b/lib/reports/pick.py index f58bda2..f14ab4d 100644 --- a/lib/reports/pick.py +++ b/lib/reports/pick.py @@ -95,12 +95,12 @@ def screen(service, cmd, found, group, period, shown, mask, view, page=0, note=N step = 'n' if note else 'm' # the tickers' screen, at a page button = press_button(cmd, crc, group, period, shown, mask) rows = [] - if view == 'g': # the treemap's menu, by name in rows of three: a choice redoes the chart and stays; Done + if view == 'g': # the treemap's menu, by name in rows of three: a choice redoes the chart and stays; OK rows = buttons.chunks([button(name.capitalize(), f"q{page}", name == holdings.grouping(group), group=key) for key, name in sorted(holdings.GROUPS.items(), key=lambda item: item[1])], 3) if len(rows[-1]) == 3: rows.append([]) - rows[-1].append(button('Done', f"p{page}")) # last, on a row of its own where the last is full + rows[-1].append(button('OK', f"p{page}")) # last, on a row of its own where the last is full return ['Grouped by'], rows if cmd == 'c' and view == 'p': # the periods, and + to the tickers rows.append([button(buttons.PERIODS[key][0], 'p0', key == period, period=key) for key in buttons.BUTTONS['c']] + diff --git a/tests/test_reports.py b/tests/test_reports.py index 7ee5a8e..def6b6a 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -1168,21 +1168,21 @@ def test_portfolios_come_four_to_a_row(self): def test_the_group_menu_and_another_portfolio_redraw_the_answer(self): """The Grouped by button opens a menu of the five groupings, the one shown marked; a choice redoes the chart in - place and keeps the menu, Done (the last button) returns to the portfolio buttons; another portfolio's button + place and keeps the menu, OK (the last button) returns to the portfolio buttons; another portfolio's button redoes it too.""" self.portfolios('Amy', 'Zed') self.command('.treemap') kind, lines, rows = self.tap('Grouped by: Industry β–Ύ') self.assertEqual((kind, self.labels(rows)), ('buttons', - [['Country', 'Currency', '● Industry'], ['Market', 'Sector', 'Done']])) + [['Country', 'Currency', '● Industry'], ['Market', 'Sector', 'OK']])) kind, lines, rows = self.tap('Sector') self.assertEqual((kind, lines[0], self.labels(rows)), ('photo', 'Amy treemap by sector', - [['Country', 'Currency', 'Industry'], ['Market', '● Sector', 'Done']])) # the menu stays + [['Country', 'Currency', 'Industry'], ['Market', '● Sector', 'OK']])) # the menu stays self.assertEqual(self.toasts[-1], ('Updating…', False)) kind, lines, rows = self.tap('Currency') self.assertEqual((kind, lines[0], self.labels(rows)), ('photo', 'Amy treemap by currency', - [['Country', '● Currency', 'Industry'], ['Market', 'Sector', 'Done']])) - kind, lines, rows = self.tap('Done') + [['Country', '● Currency', 'Industry'], ['Market', 'Sector', 'OK']])) + kind, lines, rows = self.tap('OK') self.assertEqual((kind, lines[0], self.labels(rows)), ('photo', 'Amy treemap by currency', [['● Amy', 'Zed'], ['Grouped by: Currency β–Ύ']])) kind, lines, rows = self.tap('Zed') @@ -1190,14 +1190,14 @@ def test_the_group_menu_and_another_portfolio_redraw_the_answer(self): [['Amy', '● Zed'], ['Grouped by: Currency β–Ύ']])) def test_the_group_menu_has_a_button_for_each_grouping(self): - """The Grouped by menu is made from GROUPS, by name in rows of three, Done last: a new grouping + """The Grouped by menu is made from GROUPS, by name in rows of three, OK last: a new grouping gets its button (the five were placed by hand).""" menu = lambda: [[('● ' if button.active else '') + button.label for button in row] for row in pick.screen(TELEGRAM, 't', ['Amy', 'Zed'], '0', '0', 1, 0, 'g')[1]] - self.assertEqual(menu(), [['Country', 'Currency', '● Industry'], ['Market', 'Sector', 'Done']]) + self.assertEqual(menu(), [['Country', 'Currency', '● Industry'], ['Market', 'Sector', 'OK']]) with mock.patch.dict(holdings.GROUPS, {'x': 'exchange'}): self.assertEqual(menu(), [['Country', 'Currency', 'Exchange'], ['● Industry', 'Market', 'Sector'], - ['Done']]) + ['OK']]) def test_a_holdings_answer_keeps_its_buttons(self): """.holdings: another portfolio's list as text in place of the first's, the portfolio buttons still under it.""" From 24742c3c387268c109daff0f20ef08fbf9936eae Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 00:13:34 +0000 Subject: [PATCH 25/25] CLAUDE.md: drop the pull-request rules, so Claude Code's own PR and CI controls apply Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01JVEkZofdPjQ4jtXFKipeUX --- CLAUDE.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 68386a6..23f2672 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -3,7 +3,7 @@ ## 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 -b`), 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. If a PR from the work branch is open, update its title and description in the same iteration (see below). +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. ## The work branch is the dev branch @@ -11,12 +11,6 @@ The work branch named in the session instructions (`claude/...`) is the persiste - **Never rewrite the work branch's history, and never reset the branch after a merge**: 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, leave the branch as it is. - If the owner wants a fixed name such as `dev`, they will say so; only push to another branch with their explicit permission. -## Pull requests from the work branch -An open PR takes in every commit pushed to its branch, so a PR opened for one small change can merge far more under its old title (#35 was titled as a CI tweak and merged 14 review fixes). The merge commit on `main` keeps that title for good. -- Before each push, check for an open PR from the work branch. If there is one, update its title and description so they cover every commit it now holds, not only the change it was opened for. -- Never make such a PR mergeable (resolve its conflict, merge `main` into it, enable auto-merge) while its title or description is out of date. -- To merge: `main` takes only a branch that is up to date with it and green on every CI job. Merge `origin/main` into the work branch (a merge commit, never a rebase), run the tests, push, wait for CI on that head, then merge the PR with a merge commit. - ## 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, and its `docker` job builds the image (`Dockerfile`, Debian's packages), imports every module in it, draws a chart in