From 7535f6ffaf2aa6ea3648cf5c3863e98eebf1e111 Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Tue, 24 Feb 2026 01:38:06 +0200 Subject: [PATCH 01/13] fix(security): fix SQL injections and zero divisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - tokensdb.py: parameterize all queries in get_token_counts, get_token_balances, get_last_timestamp_by_token; add empty guards; unify Value alias casing - market.py: whitelist table names in drop_duplicate; replace N+1 loop in get_last_market with single JOIN query; fix %f→%d - tools.py: guard zero division in convert_price_to_target_currency, convert_dataframe_prices_historical, calculate_crypto_rate - 3_Operations.py: fix swapped Streamlit keys swap_amount_from/to; guard rows_selected()[0] against None; protect all rate divisions with replace(0, pd.NA); guard swap_perf rate_now==0 --- app_pages/3_Operations.py | 22 ++++++---- modules/database/market.py | 36 ++++++---------- modules/database/tokensdb.py | 82 +++++++++++++----------------------- modules/tools.py | 6 ++- 4 files changed, 61 insertions(+), 85 deletions(-) diff --git a/app_pages/3_Operations.py b/app_pages/3_Operations.py index f7f7e41..6881d73 100644 --- a/app_pages/3_Operations.py +++ b/app_pages/3_Operations.py @@ -197,10 +197,10 @@ def swap_add() -> None: with col_amount: swap_amount_from = st.number_input( - "From Amount", min_value=0.0, format="%.8g", key="swap_amount_to" + "From Amount", min_value=0.0, format="%.8g", key="swap_amount_from" ) swap_amount_to = st.number_input( - "To Amount", min_value=0.0, format="%.8g", key="swap_amount_from" + "To Amount", min_value=0.0, format="%.8g", key="swap_amount_to" ) with col_portfolio: @@ -489,7 +489,8 @@ def buy_edit(): Shows edit dialog if a row is selected. Otherwise displays a warning toast. """ - rowidx = rows_selected(st.session_state.buyselection)[0] + _sel = rows_selected(st.session_state.buyselection) + rowidx = _sel[0] if _sel is not None else None if rowidx is None: st.toast("Please select a row", icon=":material/warning:") else: @@ -503,7 +504,8 @@ def buy_delete(): Otherwise displays a warning toast. """ logger.debug("Delete row") - rowidx = rows_selected(st.session_state.buyselection)[0] + _sel = rows_selected(st.session_state.buyselection) + rowidx = _sel[0] if _sel is not None else None if rowidx is None: st.toast("Please select a row", icon=":material/warning:") else: @@ -611,6 +613,8 @@ def calc_perf(df: pd.DataFrame, COL_TOken: str, col_rate: str) -> pd.DataFrame: def swap_perf(rate_swap, rate_now) -> float: if rate_swap is None or rate_now is None: return None + if rate_now == 0: + return None return ((rate_swap * 100) / rate_now) - 100 @@ -653,7 +657,7 @@ def build_buy_dataframe( local_timezone = tzlocal.get_localzone() logger.debug("Timezone locale: %s", local_timezone) df["Date"] = df["Date"].dt.tz_convert(local_timezone).dt.tz_localize(None) - df["Buy Rate"] = df["From"] / df["To"] + df["Buy Rate"] = df["From"] / df["To"].replace(0, pd.NA) # Historical conversion of From column if convert_from and not df.empty: @@ -701,7 +705,7 @@ def build_buy_dataframe( if (convert_from or convert_to) and not df.empty: from_col = f"From ({convert_from})" if convert_from else "From" to_col = f"To ({convert_to})" if convert_to else "To" - df["Converted Rate"] = df[from_col] / df[to_col] + df["Converted Rate"] = df[from_col] / df[to_col].replace(0, pd.NA) # Current Rate and Perf. — use converted units when active if (convert_from or convert_to) and not df.empty: @@ -727,7 +731,7 @@ def build_buy_dataframe( ), axis=1, ) - df["Perf."] = ((df["Current Rate"] * 100) / df["Converted Rate"]) - 100 + df["Perf."] = ((df["Current Rate"] * 100) / df["Converted Rate"].replace(0, pd.NA)) - 100 else: if use_api: df = calc_perf_api( @@ -865,7 +869,7 @@ def build_swap_dataframe( logger.debug("Timezone locale: %s", local_timezone) df["Date"] = df["Date"].dt.tz_convert(local_timezone).dt.tz_localize(None) - df["Swap Rate"] = df["To Amount"] / df["From Amount"] + df["Swap Rate"] = df["To Amount"] / df["From Amount"].replace(0, pd.NA) # Historical conversion of From Amount column if convert_from and not df.empty: @@ -913,7 +917,7 @@ def build_swap_dataframe( if (convert_from or convert_to) and not df.empty: from_col = f"From Amount ({convert_from})" if convert_from else "From Amount" to_col = f"To Amount ({convert_to})" if convert_to else "To Amount" - df["Swap Rate"] = df[to_col] / df[from_col] + df["Swap Rate"] = df[to_col] / df[from_col].replace(0, pd.NA) # Current Rate and Perf. — use converted units when active if use_api: diff --git a/modules/database/market.py b/modules/database/market.py index ccd471d..0f12626 100644 --- a/modules/database/market.py +++ b/modules/database/market.py @@ -144,28 +144,17 @@ def get_last_market(self) -> pd.DataFrame: DataFrame with latest token prices or None if empty """ logger.debug("Get last market") - tokens_list = self.get_tokens() - if not tokens_list: - logger.warning("No tokens available") - return None with sqlite3.connect(self.db_path) as con: - market_data = [] - for token in tokens_list: - df = pd.read_sql_query( - "SELECT timestamp, price FROM Market WHERE token = ? ORDER BY timestamp DESC LIMIT 1", - con, - params=(token,), - ) - if df.empty: - continue - market_data.append( - { - "token": token, - "timestamp": df["timestamp"][0], - "value": df["price"][0], - } - ) - market_df = pd.DataFrame(market_data) + market_df = pd.read_sql_query( + "SELECT m.token, m.timestamp, m.price AS value " + "FROM Market m " + "INNER JOIN (SELECT token, MAX(timestamp) AS ts FROM Market GROUP BY token) mx " + " ON m.token = mx.token AND m.timestamp = mx.ts", + con, + ) + if market_df.empty: + logger.warning("No tokens available") + return None market_df.set_index("token", inplace=True) logger.debug("Last Market get size: %d", len(market_df)) logger.debug("Last Market get:\n%s", market_df) @@ -265,10 +254,13 @@ def drop_duplicate(self, table: str): table: Table name """ logger.debug("Drop duplicate from %s", table) + _ALLOWED_TABLES = {"TokensDatabase", "Market", "Currency", "Swaps"} + if table not in _ALLOWED_TABLES: + raise ValueError(f"Table non autorisée : {table}") with sqlite3.connect(self.db_path) as con: df = pd.read_sql_query(f"SELECT * from {table};", con) dupcount = df.duplicated().sum() - logger.debug("Found %d rows with %f duplicated rows", len(df), dupcount) + logger.debug("Found %d rows with %d duplicated rows", len(df), dupcount) if dupcount > 0: logger.debug("Found %d duplicated rows. Dropping...", dupcount) df.drop_duplicates(inplace=True) diff --git a/modules/database/tokensdb.py b/modules/database/tokensdb.py index 761fa89..92cc0e3 100644 --- a/modules/database/tokensdb.py +++ b/modules/database/tokensdb.py @@ -115,31 +115,16 @@ def get_token_counts( """ logger.debug("Get counts for token %s", token) with sqlite3.connect(self.db_path) as con: - if from_timestamp is not None and to_timestamp is not None: - query = ( - f"SELECT timestamp AS Date, count AS Count FROM TokensDatabase " - f"WHERE token = '{token}' AND timestamp >= {from_timestamp} " - f"AND timestamp <= {to_timestamp}" - ) - df = pd.read_sql_query(query, con) - elif from_timestamp is not None: - query = ( - f"SELECT timestamp AS Date, count AS Count FROM TokensDatabase " - f"WHERE token = '{token}' AND timestamp >= {from_timestamp}" - ) - df = pd.read_sql_query(query, con) - elif to_timestamp is not None: - query = ( - f"SELECT timestamp AS Date, count AS Count FROM TokensDatabase " - f"WHERE token = '{token}' AND timestamp <= {to_timestamp}" - ) - df = pd.read_sql_query(query, con) - else: - query = ( - f"SELECT timestamp AS Date, count AS Count FROM TokensDatabase " - f"WHERE token = '{token}'" - ) - df = pd.read_sql_query(query, con) + conditions = "WHERE token = ?" + params: list = [token] + if from_timestamp is not None: + conditions += " AND timestamp >= ?" + params.append(from_timestamp) + if to_timestamp is not None: + conditions += " AND timestamp <= ?" + params.append(to_timestamp) + query = f"SELECT timestamp AS Date, count AS Count FROM TokensDatabase {conditions}" + df = pd.read_sql_query(query, con, params=params) if df.empty: logger.warning("No count data found for token %s in database", token) return None @@ -158,33 +143,19 @@ def get_token_balances( """Get the balances of a token through time""" logger.debug("Get balances for token %s", token) with sqlite3.connect(self.db_path) as con: - if from_timestamp is not None and to_timestamp is not None: - query = ( - f"SELECT timestamp AS Date, price*count AS 'Value', count AS Count " - f"FROM TokensDatabase WHERE token = '{token}' " - f"AND timestamp >= {from_timestamp} AND timestamp <= {to_timestamp}" - ) - df = pd.read_sql_query(query, con) - elif from_timestamp is not None: - query = ( - f"SELECT timestamp AS Date, price*count AS 'Value', count AS Count " - f"FROM TokensDatabase WHERE token = '{token}' " - f"AND timestamp >= {from_timestamp}" - ) - df = pd.read_sql_query(query, con) - elif to_timestamp is not None: - query = ( - f"SELECT timestamp AS Date, price*count AS 'value', count AS Count " - f"FROM TokensDatabase WHERE token = '{token}' " - f"AND timestamp <= {to_timestamp}" - ) - df = pd.read_sql_query(query, con) - else: - query = ( - f"SELECT timestamp AS Date, price*count AS 'value', count AS Count " - f"FROM TokensDatabase WHERE token = '{token}'" - ) - df = pd.read_sql_query(query, con) + conditions = "WHERE token = ?" + params: list = [token] + if from_timestamp is not None: + conditions += " AND timestamp >= ?" + params.append(from_timestamp) + if to_timestamp is not None: + conditions += " AND timestamp <= ?" + params.append(to_timestamp) + query = ( + f"SELECT timestamp AS Date, price*count AS Value, count AS Count " + f"FROM TokensDatabase {conditions}" + ) + df = pd.read_sql_query(query, con, params=params) if df.empty: logger.warning("No data found for token %s in database", token) return None @@ -232,14 +203,19 @@ def get_last_timestamp(self) -> int: df = pd.read_sql_query( "SELECT MAX(timestamp) as timestamp from TokensDatabase;", con ) + if df.empty: + return None return df["timestamp"][0] def get_last_timestamp_by_token(self, token: str) -> int: with sqlite3.connect(self.db_path) as con: df = pd.read_sql_query( - f"SELECT MAX(timestamp) as timestamp from TokensDatabase WHERE token = '{token}';", + "SELECT MAX(timestamp) as timestamp from TokensDatabase WHERE token = ?;", con, + params=(token,), ) + if df.empty: + return None return df["timestamp"][0] def drop_duplicate(self): diff --git a/modules/tools.py b/modules/tools.py index 13a048e..eb0f555 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -94,6 +94,8 @@ def convert_price_to_target_currency( # Conversion EUR → USD elif source_currency == "EUR" and target_currency == "USD": + if not usd_to_eur_rate: + return value return value / usd_to_eur_rate else: @@ -189,7 +191,7 @@ def convert_dataframe_prices_historical( if source_currency == "USD" and target_currency == "EUR": converted = merged["price_value"] * merged["rate"] else: # EUR → USD - converted = merged["price_value"] / merged["rate"] + converted = merged["price_value"] / merged["rate"].replace(0, pd.NA) # Remettre les valeurs converties dans le DataFrame original (même ordre d'index) df[price_column] = converted.values @@ -554,6 +556,8 @@ def calculate_crypto_rate( value_b = __interpolate_token(token_b, timestamp, dbfile) if value_a is None or value_b is None: return None + if value_b == 0: + return None rate = value_a / value_b logger.debug("Calculate crypto rate - 1 %s = %f %s", token_a, rate, token_b) return rate From dcf335221fa646645b2db2cb2efc19f5be1fe9bb Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Tue, 24 Feb 2026 01:45:10 +0200 Subject: [PATCH 02/13] fix: prevent division by zero and fix swapped Streamlit keys - Fix inverted keys in swap_edit_dialog (swap_edit_amount_from/to) - Guard calc_perf and build_buy_avg_table against zero buy rates (airdrops) - Guard Timeframe Low/High metrics in Graphs against zero prices - Guard fiat rate inversion in batch_convert_historical_api - Use module logger instead of root logging in market.py --- app_pages/2_Graphs.py | 10 ++++++++-- app_pages/3_Operations.py | 8 ++++---- modules/database/market.py | 4 ++-- modules/tools.py | 2 +- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/app_pages/2_Graphs.py b/app_pages/2_Graphs.py index 3b81502..82f12ce 100644 --- a/app_pages/2_Graphs.py +++ b/app_pages/2_Graphs.py @@ -522,7 +522,10 @@ def draw_tab_content( "Timeframe Low", value=f"{round(min_price, 2)} {currency_symbol}", help=f"Date: {min_price_date[0]}", - delta=f"{round(((current_price - min_price) / min_price) * 100, 2)} %", + delta=( + f"{round(((current_price - min_price) / min_price) * 100, 2)} %" + if min_price != 0 else "∞ %" + ), ) with mcol4: max_price = df_view[actual_column].max() @@ -533,7 +536,10 @@ def draw_tab_content( "Timeframe High", value=f"{round(max_price, 2)} {currency_symbol}", help=f"Date: {max_price_date[0]}", - delta=f"{round(((current_price - max_price) / max_price) * 100, 2)} %", + delta=( + f"{round(((current_price - max_price) / max_price) * 100, 2)} %" + if max_price != 0 else "∞ %" + ), ) col1, col2 = st.columns([3, 1]) diff --git a/app_pages/3_Operations.py b/app_pages/3_Operations.py index 6881d73..aa7f779 100644 --- a/app_pages/3_Operations.py +++ b/app_pages/3_Operations.py @@ -317,14 +317,14 @@ def swap_edit_dialog(data: dict): "From Amount", min_value=0.0, format="%.8g", - key="swap_amount_to", + key="swap_edit_amount_from", value=float(data["From Amount"]), ) swap_amount_to = st.number_input( "To Amount", min_value=0.0, format="%.8g", - key="swap_amount_from", + key="swap_edit_amount_to", value=float(data["To Amount"]), ) @@ -606,7 +606,7 @@ def calc_perf(df: pd.DataFrame, COL_TOken: str, col_rate: str) -> pd.DataFrame: else: logger.debug("Market data:\n%s", market_df) df["Current Rate"] = df[COL_TOken].map(market_df["value"].to_dict()) - df["Perf."] = ((df["Current Rate"] * 100) / df[col_rate]) - 100 + df["Perf."] = ((df["Current Rate"] * 100) / df[col_rate].replace(0, pd.NA)) - 100 return df @@ -773,7 +773,7 @@ def build_buy_avg_table(use_api: bool = False): if df.empty: return df - df["Avg. Rate"] = df["Total Bought"] / df["Tokens Obtained"] + df["Avg. Rate"] = df["Total Bought"] / df["Tokens Obtained"].replace(0, pd.NA) if use_api: prices_usd = _get_api_latest_prices() usd_to_eur = _get_api_fiat_rate() diff --git a/modules/database/market.py b/modules/database/market.py index 0f12626..24a9ef2 100644 --- a/modules/database/market.py +++ b/modules/database/market.py @@ -304,7 +304,7 @@ def __find_missing_timestamps(self) -> pd.DataFrame: df_ret = df_timestamps[ ~df_timestamps["timestamp"].isin(df_rate_timestamps["timestamp"]) ] - logging.debug("Missing timestamps: %d", len(df_ret)) + logger.debug("Missing timestamps: %d", len(df_ret)) return df_ret def update_currencies(self, debug: bool = False): @@ -339,7 +339,7 @@ def update_currencies(self, debug: bool = False): url = f"https://free.ratesdb.com/v1/rates?from=EUR&to=USD&date={date}" response = requests.get(url, timeout=10) if response.status_code != 200: - logging.error( + logger.error( "Error updating currencies. Code: %d", response.status_code ) time.sleep(1) diff --git a/modules/tools.py b/modules/tools.py index eb0f555..79670b0 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -808,7 +808,7 @@ def batch_convert_historical_api( fiat_df = api.get_currency() if fiat_df is not None and not fiat_df.empty: # price = usd_to_eur rate, we need price in USD: 1/rate - series_cache[token] = 1.0 / fiat_df["price"] + series_cache[token] = 1.0 / fiat_df["price"].replace(0, pd.NA) else: series_cache[token] = None else: From a8f799a57e7b7937afae4319abc65f6ee7dc9f28 Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Tue, 24 Feb 2026 01:49:57 +0200 Subject: [PATCH 03/13] fix(security): eliminate SQL injection f-strings in DB layer - Replace f-string table interpolation in drop_duplicate() with a hardcoded query dict (_TABLE_QUERIES), making injection structurally impossible regardless of validation - Remove f-strings from get_token_counts() and get_token_balances() where conditions were interpolated; user data was already parameterized but the pattern was unsafe --- modules/database/market.py | 11 ++++++++--- modules/database/tokensdb.py | 6 +++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/modules/database/market.py b/modules/database/market.py index 24a9ef2..6664fac 100644 --- a/modules/database/market.py +++ b/modules/database/market.py @@ -254,11 +254,16 @@ def drop_duplicate(self, table: str): table: Table name """ logger.debug("Drop duplicate from %s", table) - _ALLOWED_TABLES = {"TokensDatabase", "Market", "Currency", "Swaps"} - if table not in _ALLOWED_TABLES: + _TABLE_QUERIES = { + "TokensDatabase": "SELECT * FROM TokensDatabase;", + "Market": "SELECT * FROM Market;", + "Currency": "SELECT * FROM Currency;", + "Swaps": "SELECT * FROM Swaps;", + } + if table not in _TABLE_QUERIES: raise ValueError(f"Table non autorisée : {table}") with sqlite3.connect(self.db_path) as con: - df = pd.read_sql_query(f"SELECT * from {table};", con) + df = pd.read_sql_query(_TABLE_QUERIES[table], con) dupcount = df.duplicated().sum() logger.debug("Found %d rows with %d duplicated rows", len(df), dupcount) if dupcount > 0: diff --git a/modules/database/tokensdb.py b/modules/database/tokensdb.py index 92cc0e3..3ab17c8 100644 --- a/modules/database/tokensdb.py +++ b/modules/database/tokensdb.py @@ -123,7 +123,7 @@ def get_token_counts( if to_timestamp is not None: conditions += " AND timestamp <= ?" params.append(to_timestamp) - query = f"SELECT timestamp AS Date, count AS Count FROM TokensDatabase {conditions}" + query = "SELECT timestamp AS Date, count AS Count FROM TokensDatabase " + conditions df = pd.read_sql_query(query, con, params=params) if df.empty: logger.warning("No count data found for token %s in database", token) @@ -152,8 +152,8 @@ def get_token_balances( conditions += " AND timestamp <= ?" params.append(to_timestamp) query = ( - f"SELECT timestamp AS Date, price*count AS Value, count AS Count " - f"FROM TokensDatabase {conditions}" + "SELECT timestamp AS Date, price*count AS Value, count AS Count " + "FROM TokensDatabase " + conditions ) df = pd.read_sql_query(query, con, params=params) if df.empty: From e09414d609ddd05be8fe267c7efce060a0a1d881 Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Tue, 24 Feb 2026 01:55:28 +0200 Subject: [PATCH 04/13] fix: fix swap_edit crash and usd rate guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix TypeError in swap_edit() when rows_selected() returns None: check for None before calling len() - Unify usd_to_eur_rate zero-guard in convert_price_to_target_currency() to cover both USD→EUR and EUR→USD branches, with explicit warning log --- app_pages/3_Operations.py | 9 ++++----- modules/tools.py | 6 ++++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app_pages/3_Operations.py b/app_pages/3_Operations.py index aa7f779..f5b4d3d 100644 --- a/app_pages/3_Operations.py +++ b/app_pages/3_Operations.py @@ -519,13 +519,12 @@ def swap_edit(): Otherwise displays a warning toast. """ rowidx = rows_selected(st.session_state.swapselection) - if len(rowidx) > 1: + if rowidx is None: + st.toast("Please select a row", icon=":material/warning:") + elif len(rowidx) > 1: st.toast("Please select only one row", icon=":material/warning:") else: - if rowidx[0] is None: - st.toast("Please select a row", icon=":material/warning:") - else: - swap_edit_dialog(df_swap.iloc[rowidx[0]].to_dict()) + swap_edit_dialog(df_swap.iloc[rowidx[0]].to_dict()) def swap_delete(): diff --git a/modules/tools.py b/modules/tools.py index 79670b0..c8779b6 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -88,14 +88,16 @@ def convert_price_to_target_currency( usd_to_eur_rate = df_rate["price"].iloc[-1] # Ex: 0.85 (1 USD = 0.85 EUR) + if not usd_to_eur_rate: + logger.warning("Taux USD/EUR invalide: %s, retour valeur originale", usd_to_eur_rate) + return value + # Conversion USD → EUR if source_currency == "USD" and target_currency == "EUR": return value * usd_to_eur_rate # Conversion EUR → USD elif source_currency == "EUR" and target_currency == "USD": - if not usd_to_eur_rate: - return value return value / usd_to_eur_rate else: From 1ff2da0a36a3d07225310ca087c11bec88bb0c9a Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Tue, 24 Feb 2026 01:57:55 +0200 Subject: [PATCH 05/13] refactor: minor hardening from code review suggestions - Extend usd_to_eur_rate guard to also reject negative rates - Move _DROP_DUPLICATE_QUERIES to module-level constant in market.py - Add clarifying comments in tokensdb.py that conditions strings contain only SQL structure, not user data --- modules/database/market.py | 17 +++++++++-------- modules/database/tokensdb.py | 2 ++ modules/tools.py | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/modules/database/market.py b/modules/database/market.py index 6664fac..7e72a0a 100644 --- a/modules/database/market.py +++ b/modules/database/market.py @@ -19,6 +19,13 @@ logger = logging.getLogger(__name__) +_DROP_DUPLICATE_QUERIES = { + "TokensDatabase": "SELECT * FROM TokensDatabase;", + "Market": "SELECT * FROM Market;", + "Currency": "SELECT * FROM Currency;", + "Swaps": "SELECT * FROM Swaps;", +} + class Market: """Class for managing cryptocurrency market data and currency rates.""" @@ -254,16 +261,10 @@ def drop_duplicate(self, table: str): table: Table name """ logger.debug("Drop duplicate from %s", table) - _TABLE_QUERIES = { - "TokensDatabase": "SELECT * FROM TokensDatabase;", - "Market": "SELECT * FROM Market;", - "Currency": "SELECT * FROM Currency;", - "Swaps": "SELECT * FROM Swaps;", - } - if table not in _TABLE_QUERIES: + if table not in _DROP_DUPLICATE_QUERIES: raise ValueError(f"Table non autorisée : {table}") with sqlite3.connect(self.db_path) as con: - df = pd.read_sql_query(_TABLE_QUERIES[table], con) + df = pd.read_sql_query(_DROP_DUPLICATE_QUERIES[table], con) dupcount = df.duplicated().sum() logger.debug("Found %d rows with %d duplicated rows", len(df), dupcount) if dupcount > 0: diff --git a/modules/database/tokensdb.py b/modules/database/tokensdb.py index 3ab17c8..b46e54d 100644 --- a/modules/database/tokensdb.py +++ b/modules/database/tokensdb.py @@ -115,6 +115,7 @@ def get_token_counts( """ logger.debug("Get counts for token %s", token) with sqlite3.connect(self.db_path) as con: + # conditions contains only SQL structure (no user data); values are in params conditions = "WHERE token = ?" params: list = [token] if from_timestamp is not None: @@ -143,6 +144,7 @@ def get_token_balances( """Get the balances of a token through time""" logger.debug("Get balances for token %s", token) with sqlite3.connect(self.db_path) as con: + # conditions contains only SQL structure (no user data); values are in params conditions = "WHERE token = ?" params: list = [token] if from_timestamp is not None: diff --git a/modules/tools.py b/modules/tools.py index c8779b6..8497cd6 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -88,7 +88,7 @@ def convert_price_to_target_currency( usd_to_eur_rate = df_rate["price"].iloc[-1] # Ex: 0.85 (1 USD = 0.85 EUR) - if not usd_to_eur_rate: + if not usd_to_eur_rate or usd_to_eur_rate < 0: logger.warning("Taux USD/EUR invalide: %s, retour valeur originale", usd_to_eur_rate) return value From ac500d5791c1e8bf6d65bc31ba52ec7ffefd7387 Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Tue, 24 Feb 2026 02:26:02 +0200 Subject: [PATCH 06/13] refactor: apply code review fixes across 8 files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 1_Portfolios: remove unused get_portfolio_history() and TokensDatabase import - 2_Graphs: fix if/elif chain, remove unused apimarket instance, simplify redundant elif - 3_Operations: rename COL_TOken→col_token, merge draw_swap/draw_swap_arch into _draw_swap_table, add ttl=3600 to build_swap_dataframe cache - 4_Import: fix log typo, replace traceback.print_exc() with logger.exception(), remove traceback import - 6_Settings: remove redundant st.session_state.fiat_currency double source of truth - market.py: wrap response.json() in try/except ValueError - token_metadata.py: remove hardcoded __main__ block - tools.py: move sqlite3/requests imports to module level, narrow except Exception --- app_pages/1_Portfolios.py | 65 ---------------- app_pages/2_Graphs.py | 21 ++--- app_pages/3_Operations.py | 155 ++++++++++--------------------------- app_pages/4_Import.py | 7 +- app_pages/6_Settings.py | 9 +-- modules/database/market.py | 43 +++++----- modules/token_metadata.py | 41 +++------- modules/tools.py | 12 +-- 8 files changed, 91 insertions(+), 262 deletions(-) diff --git a/app_pages/1_Portfolios.py b/app_pages/1_Portfolios.py index f49a2a0..7507b70 100644 --- a/app_pages/1_Portfolios.py +++ b/app_pages/1_Portfolios.py @@ -3,11 +3,9 @@ import pandas as pd import streamlit as st -import plotly.graph_objects as go from modules.database.customdata import Customdata from modules.database.portfolios import Portfolios -from modules.database.tokensdb import TokensDatabase from modules.tools import ( create_portfolio_dataframe, update, @@ -214,69 +212,6 @@ def load_portfolios(dbfile: str) -> Portfolios: return Portfolios(dbfile) -def get_portfolio_history(portfolio_name: str, dbfile: str) -> pd.DataFrame: - """Get historical value of a portfolio over time. - - Args: - portfolio_name: Name of the portfolio - dbfile: Path to database file - - Returns: - DataFrame with timestamp index and portfolio value - """ - logger.debug("Getting history for portfolio %s", portfolio_name) - - # Get current portfolio tokens and amounts - portfolio = Portfolios(dbfile) - tokens_dict = portfolio.get_tokens(portfolio_name) - - if not tokens_dict: - logger.warning("Portfolio %s is empty", portfolio_name) - return pd.DataFrame() - - # Get historical prices from TokensDatabase - tokensdb = TokensDatabase(dbfile) - - # Query all historical data for tokens in this portfolio - import sqlite3 - - with sqlite3.connect(dbfile) as con: - # Get tokens list - tokens_list = list(tokens_dict.keys()) - tokens_placeholder = ",".join(["?" for _ in tokens_list]) - - query = f""" - SELECT timestamp, token, price - FROM TokensDatabase - WHERE token IN ({tokens_placeholder}) - ORDER BY timestamp - """ - df = pd.read_sql_query(query, con, params=tokens_list) - - if df.empty: - logger.warning("No historical data found for portfolio %s", portfolio_name) - return pd.DataFrame() - - # Convert timestamp to datetime - df["timestamp"] = pd.to_datetime(df["timestamp"], unit="s", utc=True) - df["timestamp"] = ( - df["timestamp"].dt.tz_convert(tokensdb.local_timezone).dt.tz_localize(None) - ) - - # Calculate value for each token at each timestamp - df["amount"] = df["token"].map(lambda t: float(tokens_dict.get(t, 0))) - df["value"] = df["price"] * df["amount"] - - # Group by timestamp and sum values - portfolio_value = df.groupby("timestamp")["value"].sum().reset_index() - portfolio_value.columns = ["Date", "Value"] - portfolio_value.set_index("Date", inplace=True) - portfolio_value.sort_index(inplace=True) - - logger.debug("Portfolio history shape: %s", portfolio_value.shape) - return portfolio_value - - @st.fragment def execute_search(): """Execute token search and display results. diff --git a/app_pages/2_Graphs.py b/app_pages/2_Graphs.py index 82f12ce..f4340c3 100644 --- a/app_pages/2_Graphs.py +++ b/app_pages/2_Graphs.py @@ -750,14 +750,6 @@ def build_price_tab( st.session_state.settings["coinmarketcap_token"], ) -# Initialize ApiMarket for MarketRaccoon API access -cache_file = os.path.join(st.session_state.settings["data_path"], "api_cache.json") -apimarket = ApiMarket( - st.session_state.settings["marketraccoon_url"], - api_key=st.session_state.settings.get("marketraccoon_token"), - cache_file=cache_file, -) - if "tokens" not in st.session_state: # Load saved token preferences from settings st.session_state.tokens = st.session_state.settings.get( @@ -769,17 +761,17 @@ def build_price_tab( st.title("Global") aggregater_ui() -if add_selectbox == "Assets Balances": +elif add_selectbox == "Assets Balances": logger.debug("Assets Balances") st.title("Assets Balances") build_tabs(use_api=use_api_sidebar) -if add_selectbox == "Market": +elif add_selectbox == "Market": logger.debug("Market") st.title("Market") build_tabs("Market", use_api=use_api_sidebar) -if add_selectbox == "Currency": +elif add_selectbox == "Currency": # Determine currency direction from toggle currency_inverted = st.session_state.get("currency_direction_toggle", True) @@ -810,7 +802,7 @@ def build_price_tab( # Invert prices if EUR/USD is selected if currency_inverted and currency_data is not None and not currency_data.empty: currency_data = currency_data.copy() - currency_data["price"] = 1 / currency_data["price"] + currency_data["price"] = 1 / currency_data["price"].replace(0, pd.NA) build_price_tab(currency_data, chart_title=CHART_TITLE, chart_y_label=CHART_Y_LABEL) @@ -866,8 +858,5 @@ def build_price_tab( st.info(f"Exact value: {value:.6f} {UNIT}") else: st.warning("Value is 0.0") - elif ( - st.session_state.interpolation_result is None - and "interpolation_result" in st.session_state - ): + else: st.info("No data available") diff --git a/app_pages/3_Operations.py b/app_pages/3_Operations.py index f5b4d3d..46372c2 100644 --- a/app_pages/3_Operations.py +++ b/app_pages/3_Operations.py @@ -190,8 +190,8 @@ def swap_add() -> None: date = st.date_input("Date", key="swap_date") with col_time: time = st.time_input("Time", key="swap_time") - COL_TOken, col_amount, col_portfolio = st.columns(3) - with COL_TOken: + col_token, col_amount, col_portfolio = st.columns(3) + with col_token: swap_token_from = st.text_input("From Token", key="swap_token_from") swap_token_to = st.text_input("To Token", key="swap_token_to") @@ -303,8 +303,8 @@ def swap_edit_dialog(data: dict): date = st.date_input("Date", key="swap_date", value=data["Date"]) with col_time: time = st.time_input("Time", key="swap_time", value=data["Date"]) - COL_TOken, col_amount, col_portfolio = st.columns(3) - with COL_TOken: + col_token, col_amount, col_portfolio = st.columns(3) + with col_token: swap_token_from = st.text_input( "From Token", key="swap_token_from", value=data["From Token"] ) @@ -583,12 +583,12 @@ def swap_arch_delete(): swap_arch_delete_dialog(rowidx) -def calc_perf(df: pd.DataFrame, COL_TOken: str, col_rate: str) -> pd.DataFrame: +def calc_perf(df: pd.DataFrame, col_token: str, col_rate: str) -> pd.DataFrame: """Calculate current performance metrics for operations. Args: df: DataFrame containing operations data - COL_TOken: Name of column containing token symbols + col_token: Name of column containing token symbols col_rate: Name of column containing original rates Returns: @@ -604,7 +604,7 @@ def calc_perf(df: pd.DataFrame, COL_TOken: str, col_rate: str) -> pd.DataFrame: df["Perf."] = None else: logger.debug("Market data:\n%s", market_df) - df["Current Rate"] = df[COL_TOken].map(market_df["value"].to_dict()) + df["Current Rate"] = df[col_token].map(market_df["value"].to_dict()) df["Perf."] = ((df["Current Rate"] * 100) / df[col_rate].replace(0, pd.NA)) - 100 return df @@ -796,52 +796,38 @@ def build_swap_dataframes( convert_to: str = None, use_api: bool = False, ) -> pd.DataFrame: - df1 = pd.DataFrame( - g_swaps.get_by_tag(""), - columns=[ - "id", - "timestamp", - "From Token", - "From Amount", - "From Wallet", - "To Token", - "To Amount", - "To Wallet", - "tag", - "note", - ], - ) + swaps = Swaps(db_file) + columns = [ + "id", + "timestamp", + "From Token", + "From Amount", + "From Wallet", + "To Token", + "To Amount", + "To Wallet", + "tag", + "note", + ] + df1 = pd.DataFrame(swaps.get_by_tag(""), columns=columns) if not df1.empty: - df1 = build_swap_dataframe(df1, convert_from, convert_to, use_api) + df1 = build_swap_dataframe(df1, db_file, convert_from, convert_to, use_api) else: st.info("No swap operations") - df2 = pd.DataFrame( - g_swaps.get_by_tag("archived"), - columns=[ - "id", - "timestamp", - "From Token", - "From Amount", - "From Wallet", - "To Token", - "To Amount", - "To Wallet", - "tag", - "note", - ], - ) + df2 = pd.DataFrame(swaps.get_by_tag("archived"), columns=columns) if not df2.empty: - df2 = build_swap_dataframe(df2, convert_from, convert_to, use_api) + df2 = build_swap_dataframe(df2, db_file, convert_from, convert_to, use_api) else: st.info("No archived swaps") return df1, df2 -@st.cache_data() +@st.cache_data(ttl=3600) def build_swap_dataframe( df: pd.DataFrame, + db_file: str, convert_from: str = None, convert_to: str = None, use_api: bool = False, @@ -888,7 +874,7 @@ def build_swap_dataframe( "From Token", convert_from, "timestamp", - st.session_state.settings["dbfile"], + db_file, ) # Historical conversion of To Amount column @@ -909,7 +895,7 @@ def build_swap_dataframe( "To Token", convert_to, "timestamp", - st.session_state.settings["dbfile"], + db_file, ) # Recalculate Swap Rate if conversions are active @@ -936,14 +922,13 @@ def build_swap_dataframe( ) else: now_ts = int(pd.Timestamp.now(tz="UTC").timestamp()) - dbfile = st.session_state.settings["dbfile"] df["Current Rate"] = df.apply( lambda row: ( calculate_crypto_rate( convert_from if convert_from else row["From Token"], convert_to if convert_to else row["To Token"], now_ts, - dbfile, + db_file, ) if (convert_from or row["From Token"]) != (convert_to or row["To Token"]) @@ -953,14 +938,20 @@ def build_swap_dataframe( ) # Calculate performance for each swap - df["Perf."] = swap_perf(df["Swap Rate"], df["Current Rate"]) + df["Perf."] = ((df["Swap Rate"] * 100) / df["Current Rate"].replace(0, pd.NA)) - 100 return df -def draw_swap(df: pd.DataFrame, convert_from: str = None, convert_to: str = None): +def _draw_swap_table( + df: pd.DataFrame, + selection_key: str, + empty_message: str, + convert_from: str = None, + convert_to: str = None, +): if df.empty: - st.info("No swap operations") + st.info(empty_message) else: # Apply styling based on Perf. column values using configurable thresholds green_threshold = st.session_state.settings.get( @@ -1022,76 +1013,16 @@ def color_rows(row): column_config=column_config, on_select="rerun", selection_mode="multi-row", - key="swapselection", + key=selection_key, ) -def draw_swap_arch(df: pd.DataFrame, convert_from: str = None, convert_to: str = None): - if df.empty: - st.info("No archived swaps") - else: - # Apply styling based on Perf. column values using configurable thresholds - green_threshold = st.session_state.settings.get( - "operations_green_threshold", 100 - ) - orange_threshold = st.session_state.settings.get( - "operations_orange_threshold", 50 - ) - red_threshold = st.session_state.settings.get("operations_red_threshold", 0) - - def color_rows(row): - if pd.isna(row["Perf."]): - return [""] * len(row) - elif row["Perf."] >= green_threshold: - return ["background-color: #90EE90"] * len(row) # Light green - elif row["Perf."] >= orange_threshold: - return ["background-color: #FFA500"] * len(row) # Orange - elif row["Perf."] < red_threshold: - return ["background-color: #FFB6C1"] * len(row) # Light red - else: - return [""] * len(row) - - styled_df = df.style.apply(color_rows, axis=1) - - # Build dynamic column order and config - COL_FROM_amount = ( - f"From Amount ({convert_from})" if convert_from else "From Amount" - ) - COL_TO_amount = f"To Amount ({convert_to})" if convert_to else "To Amount" - RATE_COL = "Swap Rate" - - column_order = [ - "Date", - COL_FROM_amount, - *(() if convert_from else ("From Token",)), - COL_TO_amount, - *(() if convert_to else ("To Token",)), - "From Wallet", - "To Wallet", - RATE_COL, - "Current Rate", - "Perf.", - "note", - ] +def draw_swap(df: pd.DataFrame, convert_from: str = None, convert_to: str = None): + _draw_swap_table(df, "swapselection", "No swap operations", convert_from, convert_to) - column_config = { - COL_FROM_amount: st.column_config.NumberColumn(format="%.8g"), - COL_TO_amount: st.column_config.NumberColumn(format="%.8g"), - RATE_COL: st.column_config.NumberColumn(format="%.8g"), - "Current Rate": st.column_config.NumberColumn(format="%.8g"), - "Perf.": st.column_config.NumberColumn(format="%.2f%%"), - } - st.dataframe( - styled_df, - width="stretch", - hide_index=True, - column_order=column_order, - column_config=column_config, - on_select="rerun", - selection_mode="multi-row", - key="swapachselection", - ) +def draw_swap_arch(df: pd.DataFrame, convert_from: str = None, convert_to: str = None): + _draw_swap_table(df, "swapachselection", "No archived swaps", convert_from, convert_to) g_wallets = Portfolios(st.session_state.settings["dbfile"]).get_portfolio_names() diff --git a/app_pages/4_Import.py b/app_pages/4_Import.py index e99d48d..1bcf6e1 100644 --- a/app_pages/4_Import.py +++ b/app_pages/4_Import.py @@ -1,6 +1,5 @@ import io import logging -import traceback from json import loads import pandas as pd @@ -82,7 +81,7 @@ def extract(input_data: any) -> pd.DataFrame: raise ValueError("Invalid input type") except (ValueError, KeyError, TypeError) as e: st.error("Unexpected error: " + str(e)) - traceback.print_exc() + logger.exception("Extraction failed") st.stop() output["select"] = False @@ -220,8 +219,8 @@ def clean_session_state(): else: logger.debug("File: %s - file type: %s", file.name, file.type) - if file.type == "application/vnd.ms-excel": - logger.debug("CSV file detectimport_pageed") + if file.type in ("text/csv", "application/vnd.ms-excel") or file.name.endswith(".csv"): + logger.debug("CSV file detected") input_file = process_csv(file) else: logger.debug("Image file detected") diff --git a/app_pages/6_Settings.py b/app_pages/6_Settings.py index 7d9ee35..d02cfa3 100644 --- a/app_pages/6_Settings.py +++ b/app_pages/6_Settings.py @@ -12,12 +12,6 @@ if "settings" not in st.session_state: st.session_state.settings = {} -# Initialize fiat currency in session state if not already set -if "fiat_currency" not in st.session_state: - st.session_state.fiat_currency = st.session_state.settings.get( - "fiat_currency", "EUR" - ) - @st.cache_data(ttl=30, show_spinner=False) def _check_marketraccoon(url: str, api_key: str) -> str: """Vérifie la disponibilité de l'API MarketRaccoon (résultat mis en cache 30s).""" @@ -29,6 +23,8 @@ def _check_marketraccoon(url: str, api_key: str) -> str: return "connection_error" except requests.Timeout: return "timeout" + except requests.exceptions.MissingSchema: + return "invalid_url" with st.sidebar: @@ -155,7 +151,6 @@ def _check_marketraccoon(url: str, api_key: str) -> str: ) st.session_state.settings["operations_red_threshold"] = operations_red_threshold st.session_state.settings["fiat_currency"] = fiat_currency - st.session_state.fiat_currency = fiat_currency conf = Configuration() conf.save_config(st.session_state.settings) diff --git a/modules/database/market.py b/modules/database/market.py index 7e72a0a..4e86020 100644 --- a/modules/database/market.py +++ b/modules/database/market.py @@ -78,26 +78,16 @@ def get_market(self) -> pd.DataFrame: """ logger.debug("Get market") with sqlite3.connect(self.db_path) as con: - df_tokens = pd.read_sql_query("select DISTINCT token from Market", con) - if df_tokens.empty: - return None - df_market = pd.DataFrame() - for token in df_tokens["token"]: - df = pd.read_sql_query( - "SELECT timestamp, price FROM Market WHERE token = ?", - con, - params=(token,), - ) - if df.empty: - continue - df.rename(columns={"price": token}, inplace=True) - if df_market.empty: - df_market = df - else: - df_market = df_market.merge(df, on="timestamp", how="outer") - if df_market.empty: + df_all = pd.read_sql_query( + "SELECT timestamp, token, price FROM Market ORDER BY timestamp", con + ) + if df_all.empty: return None - + df_market = df_all.pivot_table( + index="timestamp", columns="token", values="price", aggfunc="last" + ) + df_market.columns.name = None + df_market.reset_index(inplace=True) df_market["timestamp"] = ( pd.to_datetime(df_market["timestamp"], unit="s", utc=True) .dt.tz_convert(self.local_timezone) @@ -126,10 +116,10 @@ def get_token_market( with sqlite3.connect(self.db_path) as con: query = "SELECT timestamp AS Date, price AS Price FROM Market WHERE token = ?" params = [token] - if from_timestamp: + if from_timestamp is not None: query += " AND timestamp >= ?" params.append(from_timestamp) - if to_timestamp: + if to_timestamp is not None: query += " AND timestamp <= ?" params.append(to_timestamp) query += " ORDER BY timestamp" @@ -349,8 +339,15 @@ def update_currencies(self, debug: bool = False): "Error updating currencies. Code: %d", response.status_code ) time.sleep(1) - raise ValueError("Error updating currencies") - resp = response.json() + continue + try: + resp = response.json() + except ValueError: + logger.error( + "Invalid JSON response from currency API for date %s", date + ) + time.sleep(1) + continue logger.debug( "Rate Timestamp: %d - Rate: %f", diff --git a/modules/token_metadata.py b/modules/token_metadata.py index 9fc346b..6ab290d 100644 --- a/modules/token_metadata.py +++ b/modules/token_metadata.py @@ -248,7 +248,18 @@ def filter_active_tokens(self, tokens: List[str]) -> List[str]: Returns: Liste filtrée contenant uniquement les tokens actifs """ - return [token for token in tokens if self.is_token_active(token)] + if not tokens: + return [] + placeholders = ",".join(["?" for _ in tokens]) + with sqlite3.connect(self.db_path) as conn: + cursor = conn.cursor() + cursor.execute( + f"SELECT token FROM TokenMetadata WHERE token IN ({placeholders})" + f" AND status = ?", + (*tokens, TokenStatus.DELISTED.value), + ) + delisted = {row[0] for row in cursor.fetchall()} + return [token for token in tokens if token not in delisted] def get_mr_id(self, token: str) -> Optional[int]: """Get MarketRaccoon ID for a token symbol. @@ -456,31 +467,3 @@ def get_all_metadata(self) -> List[Dict]: ORDER BY status, token""" ) return [dict(row) for row in cursor.fetchall()] - - -# Exemple d'utilisation -if __name__ == "__main__": - manager = TokenMetadataManager() - - # Vérifier si un token est actif - print( - f"BTC actif: {manager.is_token_active('BTC')}" - ) # True (pas dans métadonnées = actif par défaut) - print(f"KYROS actif: {manager.is_token_active('KYROS')}") # False - print(f"MATIC délisté: {manager.is_token_delisted('MATIC')}") # True - - # Récupérer tous les tokens délistés - delisted = manager.get_delisted_tokens() - print(f"\nTokens délistés: {delisted}") - - # Récupérer les infos d'un token - info = manager.get_token_info("MATIC") - if info: - print("\nInfo MATIC:") - for key, value in info.items(): - print(f" {key}: {value}") - - # Filtrer une liste de tokens - all_tokens = ["BTC", "ETH", "MATIC", "KYROS", "SOL"] - active_only = manager.filter_active_tokens(all_tokens) - print(f"\nTokens actifs parmi {all_tokens}: {active_only}") diff --git a/modules/tools.py b/modules/tools.py index 8497cd6..404c787 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -11,10 +11,12 @@ import logging import os import shutil +import sqlite3 import traceback import numpy as np import pandas as pd +import requests import streamlit as st from modules.database.customdata import Customdata @@ -106,7 +108,7 @@ def convert_price_to_target_currency( ) return value - except Exception as e: + except (requests.exceptions.RequestException, ValueError, KeyError) as e: logger.error("Erreur conversion devise: %s", e) return value @@ -436,10 +438,8 @@ def load_settings(settings: dict): logger.error("Unable to create db directory %s: %s", db_dir, e) try: - import sqlite3 as _sqlite - # Attempt to open/create the database file - with _sqlite.connect(st.session_state.settings["dbfile"]) as _conn: + with sqlite3.connect(st.session_state.settings["dbfile"]) as _conn: pass except Exception as e: logger.error( @@ -449,7 +449,7 @@ def load_settings(settings: dict): ) fallback_db = os.path.join(os.getcwd(), "db.sqlite3") try: - with _sqlite.connect(fallback_db) as _conn: + with sqlite3.connect(fallback_db) as _conn: pass st.session_state.settings["dbfile"] = fallback_db logger.info("Falling back to dbfile: %s", fallback_db) @@ -964,7 +964,7 @@ def get_price_target(token): df["Current Rate"] = df[col_token].map(get_price_target) - df["Perf."] = ((df["Current Rate"] * 100) / df[col_rate]) - 100 + df["Perf."] = ((df["Current Rate"] * 100) / df[col_rate].replace(0, pd.NA)) - 100 return df From eb7d9fdbd442f3be56b24de8e4bc42e96a889453 Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Tue, 24 Feb 2026 10:18:34 +0200 Subject: [PATCH 07/13] fix: update .gitignore to include stats.json files and modify migration logic for db_version handling --- .gitignore | 2 ++ modules/database/apimarket.py | 6 +++--- modules/database/migrations.py | 5 ++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 0ef66e2..133e96e 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,5 @@ requirements.txt !.github/workflows/ .ruff_cache/ .claude/ +stats.json +stats.json.lock diff --git a/modules/database/apimarket.py b/modules/database/apimarket.py index efa075e..644c47a 100644 --- a/modules/database/apimarket.py +++ b/modules/database/apimarket.py @@ -357,16 +357,16 @@ def get_cryptocurrency_market( if self.api_key: headers["X-API-Key"] = self.api_key + first_page = True while next_url: logger.debug("Fetching page: %s", next_url) request = requests.get( next_url, - params=params - if next_url == self.url + "/api/v1/cryptocurrency" - else None, + params=params if first_page else None, headers=headers, timeout=10, ) + first_page = False if request.status_code == 200: data = request.json() diff --git a/modules/database/migrations.py b/modules/database/migrations.py index 17f7f27..89f61e0 100644 --- a/modules/database/migrations.py +++ b/modules/database/migrations.py @@ -222,5 +222,8 @@ def run_migrations(db_path: str) -> None: logger.info("Application de la migration v%d…", version) with sqlite3.connect(db_path) as conn: MIGRATIONS[version](conn) - _set_db_version(db_path, version) + conn.execute( + "INSERT OR REPLACE INTO Customdata (name, value, type) VALUES ('db_version', ?, 'int')", + (str(version),), + ) logger.info("Migration v%d appliquée — db_version = %d", version, version) From 7a4aef3be1246d59ac2600f394344a2da3790508 Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Tue, 24 Feb 2026 11:21:03 +0200 Subject: [PATCH 08/13] refactor: harden apimarket and migrations modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit apimarket.py: - Wrap all requests.get() calls in try/except RequestException - Add _MAX_PAGES=1000 guard on pagination loops with warning log - Rename variable `request` → `response` (requests convention) - Replace all inplace=True with explicit df = df.method() assignments - Extract _SOURCE_CURRENCY constant, replace hardcoded "USD" migrations.py: - Add try/except sqlite3.Error in _ensure_customdata, _get_db_version, _set_db_version with re-raise for clear failure messages - Handle ValueError in _get_db_version for corrupted db_version values - Replace silent pass in _migrate_v2/_migrate_v5 with logger.debug() - Extract _V2_COLUMNS dict to make f-string SQL intent explicit --- modules/database/apimarket.py | 195 ++++++++++++++++++++------------- modules/database/migrations.py | 68 ++++++++---- 2 files changed, 162 insertions(+), 101 deletions(-) diff --git a/modules/database/apimarket.py b/modules/database/apimarket.py index 644c47a..c7c2927 100644 --- a/modules/database/apimarket.py +++ b/modules/database/apimarket.py @@ -17,6 +17,9 @@ logger = logging.getLogger(__name__) +_SOURCE_CURRENCY = "USD" +_MAX_PAGES = 1000 + class ApiMarket: """Client for MarketRaccoon API to fetch fiat currency exchange rates. @@ -56,13 +59,17 @@ def get_fiat_latest_rate(self) -> pd.DataFrame: if self.api_key: headers["X-API-Key"] = self.api_key - request = requests.get( - self.url + "/api/v1/fiat/latest", - headers=headers, - timeout=10, - ) - if request.status_code == 200: - data = request.json() + try: + response = requests.get( + self.url + "/api/v1/fiat/latest", + headers=headers, + timeout=10, + ) + except requests.exceptions.RequestException as e: + logger.error("API request failed: %s", e) + return None + if response.status_code == 200: + data = response.json() # L'API retourne un array, pas un objet if not data: # Si la liste est vide return None @@ -73,17 +80,17 @@ def get_fiat_latest_rate(self) -> pd.DataFrame: df["date"] = ( df["date"].dt.tz_convert(self.local_timezone).dt.tz_localize(None) ) - df.rename(columns={"date": "Date", "eur": "price"}, inplace=True) - df.set_index("Date", inplace=True) - df.sort_index(inplace=True) + df = df.rename(columns={"date": "Date", "eur": "price"}) + df = df.set_index("Date") + df = df.sort_index() return df - if request.status_code == 204: + if response.status_code == 204: # Pas de données disponibles logger.info("No fiat data available (204)") return None - logger.error("Error fetching fiat rates: %s", request.status_code) + logger.error("Error fetching fiat rates: %s", response.status_code) return None def get_fiat_latest_rate_cached(self) -> Optional[pd.DataFrame]: @@ -148,7 +155,7 @@ def _deserialize_latest_rate(self, cached_data: dict) -> pd.DataFrame: ] ) - df.set_index("Date", inplace=True) + df = df.set_index("Date") return df def get_currency(self, timestamp: int = None) -> pd.DataFrame: @@ -178,15 +185,19 @@ def get_currency(self, timestamp: int = None) -> pd.DataFrame: if self.api_key: headers["X-API-Key"] = self.api_key - request = requests.get( - self.url + "/api/v1/fiat", - params={"date": date_str}, - headers=headers, - timeout=10, - ) + try: + response = requests.get( + self.url + "/api/v1/fiat", + params={"date": date_str}, + headers=headers, + timeout=10, + ) + except requests.exceptions.RequestException as e: + logger.error("API request failed: %s", e) + return None - if request.status_code == 200: - data = request.json() + if response.status_code == 200: + data = response.json() results = data.get("results", []) if not results: @@ -198,18 +209,18 @@ def get_currency(self, timestamp: int = None) -> pd.DataFrame: df["date"] = ( df["date"].dt.tz_convert(self.local_timezone).dt.tz_localize(None) ) - df.rename(columns={"date": "Date", "eur": "price"}, inplace=True) - df.set_index("Date", inplace=True) + df = df.rename(columns={"date": "Date", "eur": "price"}) + df = df.set_index("Date") logger.info( "Retrieved interpolated fiat rate for timestamp %d", timestamp ) return df - elif request.status_code == 204: + elif response.status_code == 204: logger.info("No fiat data available (204)") return None else: - logger.error("Error fetching fiat data: %s", request.status_code) + logger.error("Error fetching fiat data: %s", response.status_code) return None # Otherwise, fetch all data with pagination @@ -222,12 +233,18 @@ def get_currency(self, timestamp: int = None) -> pd.DataFrame: if self.api_key: headers["X-API-Key"] = self.api_key - while next_url: + pages_fetched = 0 + while next_url and pages_fetched < _MAX_PAGES: logger.debug("Fetching page: %s", next_url) - request = requests.get(next_url, headers=headers, timeout=10) + try: + response = requests.get(next_url, headers=headers, timeout=10) + except requests.exceptions.RequestException as e: + logger.error("API request failed: %s", e) + return None + pages_fetched += 1 - if request.status_code == 200: - data = request.json() + if response.status_code == 200: + data = response.json() results = data.get("results", []) all_results.extend(results) @@ -239,13 +256,16 @@ def get_currency(self, timestamp: int = None) -> pd.DataFrame: len(results), len(all_results), ) - elif request.status_code == 204: + elif response.status_code == 204: logger.info("No fiat data available (204)") return None else: - logger.error("Error fetching fiat data: %s", request.status_code) + logger.error("Error fetching fiat data: %s", response.status_code) return None + if pages_fetched >= _MAX_PAGES: + logger.warning("Pagination limit reached for fiat data") + # Convert to DataFrame if not all_results: logger.info("No fiat data retrieved") @@ -254,9 +274,9 @@ def get_currency(self, timestamp: int = None) -> pd.DataFrame: df = pd.DataFrame(all_results) df["date"] = pd.to_datetime(df["date"], utc=True) df["date"] = df["date"].dt.tz_convert(self.local_timezone).dt.tz_localize(None) - df.rename(columns={"date": "Date", "eur": "price"}, inplace=True) - df.set_index("Date", inplace=True) - df.sort_index(inplace=True) + df = df.rename(columns={"date": "Date", "eur": "price"}) + df = df.set_index("Date") + df = df.sort_index() logger.info("Retrieved %d total fiat records", len(df)) return df @@ -281,15 +301,19 @@ def get_coins(self, symbols: list = None) -> pd.DataFrame: if self.api_key: headers["X-API-Key"] = self.api_key - request = requests.get( - self.url + "/api/v1/coins", - params=params, - headers=headers, - timeout=10, - ) + try: + response = requests.get( + self.url + "/api/v1/coins", + params=params, + headers=headers, + timeout=10, + ) + except requests.exceptions.RequestException as e: + logger.error("API request failed: %s", e) + return None - if request.status_code == 200: - data = request.json() + if response.status_code == 200: + data = response.json() results = data.get("results", []) if not results: @@ -298,11 +322,11 @@ def get_coins(self, symbols: list = None) -> pd.DataFrame: df = pd.DataFrame(results) return df - elif request.status_code == 204: + elif response.status_code == 204: logger.info("No coins data available (204)") return None else: - logger.error("Error fetching coins: %s", request.status_code) + logger.error("Error fetching coins: %s", response.status_code) return None def get_cryptocurrency_market( @@ -358,18 +382,24 @@ def get_cryptocurrency_market( headers["X-API-Key"] = self.api_key first_page = True - while next_url: + pages_fetched = 0 + while next_url and pages_fetched < _MAX_PAGES: logger.debug("Fetching page: %s", next_url) - request = requests.get( - next_url, - params=params if first_page else None, - headers=headers, - timeout=10, - ) + try: + response = requests.get( + next_url, + params=params if first_page else None, + headers=headers, + timeout=10, + ) + except requests.exceptions.RequestException as e: + logger.error("API request failed: %s", e) + return None first_page = False + pages_fetched += 1 - if request.status_code == 200: - data = request.json() + if response.status_code == 200: + data = response.json() results = data.get("results", []) all_results.extend(results) @@ -381,15 +411,18 @@ def get_cryptocurrency_market( len(results), len(all_results), ) - elif request.status_code == 204: + elif response.status_code == 204: logger.info("No cryptocurrency data available (204)") return None else: logger.error( - "Error fetching cryptocurrency data: %s", request.status_code + "Error fetching cryptocurrency data: %s", response.status_code ) return None + if pages_fetched >= _MAX_PAGES: + logger.warning("Pagination limit reached for cryptocurrency data") + # Convert to DataFrame if not all_results: logger.info("No cryptocurrency data retrieved") @@ -402,11 +435,11 @@ def get_cryptocurrency_market( df["last_updated"] = ( df["last_updated"].dt.tz_convert(self.local_timezone).dt.tz_localize(None) ) - df.rename(columns={"last_updated": "Date", "price": "Price"}, inplace=True) - df["source_currency"] = "USD" # MÉTADONNÉE : Prix en USD + df = df.rename(columns={"last_updated": "Date", "price": "Price"}) + df["source_currency"] = _SOURCE_CURRENCY # MÉTADONNÉE : Prix en USD df = df[["Date", "Price", "source_currency"]] # Keep only relevant columns - df.set_index("Date", inplace=True) - df.sort_index(inplace=True) + df = df.set_index("Date") + df = df.sort_index() logger.info("Retrieved %d total cryptocurrency records", len(df)) return df @@ -571,11 +604,11 @@ def _deserialize_crypto_market(self, cached_data: dict) -> pd.DataFrame: # Reconstruct DataFrame df = pd.DataFrame(records) df["date"] = pd.to_datetime(df["date"], format="ISO8601") - df.rename(columns={"date": "Date", "price": "Price"}, inplace=True) + df = df.rename(columns={"date": "Date", "price": "Price"}) # Restaurer la métadonnée de devise source (USD par défaut pour rétrocompatibilité cache) - df["source_currency"] = cached_data.get("source_currency", "USD") - df.set_index("Date", inplace=True) - df.sort_index(inplace=True) + df["source_currency"] = cached_data.get("source_currency", _SOURCE_CURRENCY) + df = df.set_index("Date") + df = df.sort_index() return df @@ -634,8 +667,8 @@ def _deserialize_latest(self, cached_data: dict) -> pd.DataFrame: """ df = pd.DataFrame(cached_data["records"]) df["date"] = pd.to_datetime(df["date"], format="ISO8601") - df.rename(columns={"date": "Date"}, inplace=True) - df.set_index("Date", inplace=True) + df = df.rename(columns={"date": "Date"}) + df = df.set_index("Date") return df def get_cryptocurrency_latest(self, symbols: list = None) -> pd.DataFrame: @@ -660,15 +693,19 @@ def get_cryptocurrency_latest(self, symbols: list = None) -> pd.DataFrame: params["symbols"] = ",".join(symbols) logger.debug("Filtering by symbols: %s", params["symbols"]) - request = requests.get( - self.url + "/api/v1/cryptocurrency/latests", - params=params, - headers=headers, - timeout=10, - ) + try: + response = requests.get( + self.url + "/api/v1/cryptocurrency/latests", + params=params, + headers=headers, + timeout=10, + ) + except requests.exceptions.RequestException as e: + logger.error("API request failed: %s", e) + return None - if request.status_code == 200: - data = request.json() + if response.status_code == 200: + data = response.json() if not data: logger.info("No latest cryptocurrency data available") @@ -679,23 +716,23 @@ def get_cryptocurrency_latest(self, symbols: list = None) -> pd.DataFrame: df["last_updated"], format="ISO8601", utc=True ) # Sort by last_updated and drop duplicates to ensure only the latest price per coin is kept - df.sort_values(by="last_updated", ascending=False, inplace=True) - df.drop_duplicates(subset="coin", keep="first", inplace=True) + df = df.sort_values(by="last_updated", ascending=False) + df = df.drop_duplicates(subset="coin", keep="first") df["last_updated"] = ( df["last_updated"] .dt.tz_convert(self.local_timezone) .dt.tz_localize(None) ) - df.rename(columns={"last_updated": "Date"}, inplace=True) - df.set_index("Date", inplace=True) + df = df.rename(columns={"last_updated": "Date"}) + df = df.set_index("Date") logger.info("Retrieved %d latest cryptocurrency records", len(df)) return df - elif request.status_code == 204: + elif response.status_code == 204: logger.info("No latest cryptocurrency data available (204)") return None else: logger.error( - "Error fetching latest cryptocurrency data: %s", request.status_code + "Error fetching latest cryptocurrency data: %s", response.status_code ) return None diff --git a/modules/database/migrations.py b/modules/database/migrations.py index 89f61e0..5b6ef57 100644 --- a/modules/database/migrations.py +++ b/modules/database/migrations.py @@ -16,33 +16,51 @@ def _ensure_customdata(db_path: str) -> None: Doit être appelé avant toute lecture de db_version. """ - with sqlite3.connect(db_path) as conn: - conn.execute( - """CREATE TABLE IF NOT EXISTS Customdata ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - name TEXT NOT NULL UNIQUE, - value TEXT NOT NULL, - type TEXT NOT NULL - )""" - ) + try: + with sqlite3.connect(db_path) as conn: + conn.execute( + """CREATE TABLE IF NOT EXISTS Customdata ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + name TEXT NOT NULL UNIQUE, + value TEXT NOT NULL, + type TEXT NOT NULL + )""" + ) + except sqlite3.Error as e: + logger.error("Erreur lors de la création de Customdata : %s", e) + raise def _get_db_version(db_path: str) -> int: """Retourne la version courante de la base (0 si absente).""" - with sqlite3.connect(db_path) as conn: - row = conn.execute( - "SELECT value FROM Customdata WHERE name = 'db_version'" - ).fetchone() - return int(row[0]) if row else 0 + try: + with sqlite3.connect(db_path) as conn: + row = conn.execute( + "SELECT value FROM Customdata WHERE name = 'db_version'" + ).fetchone() + except sqlite3.Error as e: + logger.error("Erreur lors de la lecture de db_version : %s", e) + raise + if not row: + return 0 + try: + return int(row[0]) + except ValueError: + logger.warning("db_version invalide : %s, reset à 0", row[0]) + return 0 def _set_db_version(db_path: str, version: int) -> None: """Enregistre la version courante dans Customdata.""" - with sqlite3.connect(db_path) as conn: - conn.execute( - "INSERT OR REPLACE INTO Customdata (name, value, type) VALUES ('db_version', ?, 'int')", - (str(version),), - ) + try: + with sqlite3.connect(db_path) as conn: + conn.execute( + "INSERT OR REPLACE INTO Customdata (name, value, type) VALUES ('db_version', ?, 'int')", + (str(version),), + ) + except sqlite3.Error as e: + logger.error("Erreur lors de l'écriture de db_version : %s", e) + raise def _migrate_v1(conn: sqlite3.Connection) -> None: @@ -120,14 +138,20 @@ def _migrate_v1(conn: sqlite3.Connection) -> None: ) +_V2_COLUMNS: dict = { + "mr_id": "INTEGER", + "name": "TEXT", +} + + def _migrate_v2(conn: sqlite3.Connection) -> None: """Ajout des colonnes MarketRaccoon : mr_id et name dans TokenMetadata.""" cursor = conn.cursor() - for col, typedef in [("mr_id", "INTEGER"), ("name", "TEXT")]: + for col, typedef in _V2_COLUMNS.items(): try: cursor.execute(f"ALTER TABLE TokenMetadata ADD COLUMN {col} {typedef}") except sqlite3.OperationalError: - pass # colonne déjà présente + logger.debug("Colonne %s déjà présente, skip", col) def _migrate_v3(conn: sqlite3.Connection) -> None: @@ -186,7 +210,7 @@ def _migrate_v5(conn: sqlite3.Connection) -> None: try: conn.execute("ALTER TABLE Swaps ADD COLUMN note TEXT") except sqlite3.OperationalError: - pass # colonne déjà présente + logger.debug("Colonne note déjà présente dans Swaps, skip") def _migrate_v6(conn: sqlite3.Connection) -> None: From 627144224580a234ac2d65ea17bce7c3a881b328 Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Fri, 27 Feb 2026 00:26:07 +0200 Subject: [PATCH 09/13] fix(graphs): use mraccoon_id for API price resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of resolving token_symbol → coinid via /api/v1/coins (which picks the first result and can return wrong token data), now use the mraccoon_id stored in TokenMetadata when available. Falls back to token_symbol if no mraccoon_id is configured. --- app_pages/2_Graphs.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/app_pages/2_Graphs.py b/app_pages/2_Graphs.py index f4340c3..bc8d960 100644 --- a/app_pages/2_Graphs.py +++ b/app_pages/2_Graphs.py @@ -11,6 +11,7 @@ from modules.database.portfolios import Portfolios from modules.database.tokensdb import TokensDatabase from modules.plotter import plot_as_pie +from modules.token_metadata import TokenMetadataManager from modules.tools import ( create_portfolio_dataframe, get_currency_symbol, @@ -50,6 +51,7 @@ def fetch_api_crypto_market( token_symbol: str, from_ts: int, to_ts: int, + coinid: int = None, ) -> pd.DataFrame: """Fetch cryptocurrency market data from API with Streamlit session cache. @@ -60,13 +62,14 @@ def fetch_api_crypto_market( token_symbol: Token symbol to fetch from_ts: Unix timestamp for start date to_ts: Unix timestamp for end date + coinid: Optional MarketRaccoon coin ID (prioritaire sur token_symbol) Returns: DataFrame with columns: Date (index), Price or None if empty """ api = ApiMarket(api_url, api_key=api_key, cache_file=cache_file) return api.get_cryptocurrency_market_cached( - token_symbol=token_symbol, from_timestamp=from_ts, to_timestamp=to_ts + coinid=coinid, token_symbol=token_symbol, from_timestamp=from_ts, to_timestamp=to_ts ) @@ -343,6 +346,8 @@ def draw_tab_content( cache_file = os.path.join( st.session_state.settings["data_path"], "api_cache.json" ) + metadata_manager = TokenMetadataManager(st.session_state.settings["dbfile"]) + mr_id = metadata_manager.get_mr_id(token) df_prices = fetch_api_crypto_market( api_url, api_key, @@ -350,6 +355,7 @@ def draw_tab_content( token, start_timestamp, end_timestamp, + coinid=mr_id, ) if df_prices is None: st.warning( @@ -430,8 +436,11 @@ def draw_tab_content( cache_file = os.path.join( st.session_state.settings["data_path"], "api_cache.json" ) + metadata_manager = TokenMetadataManager(st.session_state.settings["dbfile"]) + mr_id = metadata_manager.get_mr_id(token) df_view = fetch_api_crypto_market( - api_url, api_key, cache_file, token, start_timestamp, end_timestamp + api_url, api_key, cache_file, token, start_timestamp, end_timestamp, + coinid=mr_id, ) # Conversion USD → devise cible avec taux historiques From f75504ab5a1d0aae1583380988278f696436f09b Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Sat, 18 Apr 2026 00:32:24 +0300 Subject: [PATCH 10/13] refactor: apply code review fixes across portfolio and tools modules --- app_pages/0_Home.py | 2 +- app_pages/1_Portfolios.py | 2 +- app_pages/2_Graphs.py | 22 +++--- app_pages/3_Operations.py | 51 ++++++------ app_pages/4_Import.py | 8 +- code-review-report.md | 137 +++++++++++++++++++++++++++++++++ modules/aiprocessing.py | 32 +++++--- modules/database/portfolios.py | 7 +- modules/database/swaps.py | 10 +-- modules/database/tokensdb.py | 23 +++--- modules/plotter.py | 4 +- modules/tools.py | 26 ++++++- modules/utils.py | 62 ++++++++++++++- 13 files changed, 310 insertions(+), 76 deletions(-) create mode 100644 code-review-report.md diff --git a/app_pages/0_Home.py b/app_pages/0_Home.py index 3b8575a..b7c65a9 100644 --- a/app_pages/0_Home.py +++ b/app_pages/0_Home.py @@ -54,7 +54,7 @@ def plot_total_value(df: pd.DataFrame): height=400, ) - st.plotly_chart(fig, width="stretch") + st.plotly_chart(fig, use_container_width=True) st.title("Crypto Update") diff --git a/app_pages/1_Portfolios.py b/app_pages/1_Portfolios.py index 7507b70..a2003bd 100644 --- a/app_pages/1_Portfolios.py +++ b/app_pages/1_Portfolios.py @@ -103,7 +103,7 @@ def delete_token(portfolio_name: str): ) if st.button("Submit"): for token in tokens: - g_portfolios.delete_token_a(portfolio_name, token) + g_portfolios.delete_token_by_name(portfolio_name, token) # Close dialog st.rerun() diff --git a/app_pages/2_Graphs.py b/app_pages/2_Graphs.py index bc8d960..c879632 100644 --- a/app_pages/2_Graphs.py +++ b/app_pages/2_Graphs.py @@ -326,6 +326,8 @@ def draw_tab_content( token: str, start_timestamp: int, end_timestamp: int, + tokensdb: TokensDatabase, + marketdb: Market, use_api: bool = False, ): logger.debug("Draw tab content for token %s", token) @@ -460,7 +462,7 @@ def draw_tab_content( df_view = df_view.drop(columns=["source_currency"]) else: # Use local SQLite database (données en EUR) - df_view = markgetdb.get_token_market( + df_view = marketdb.get_token_market( token, start_timestamp, end_timestamp ) @@ -574,6 +576,11 @@ def build_tabs(section: str = "Assets Balances", use_api: bool = False): end_timestamp = toTimestamp_A( st.session_state.enddate, pd.to_datetime("23:59:59").time() ) + tokensdb = TokensDatabase(st.session_state.settings["dbfile"]) + marketdb = Market( + st.session_state.settings["dbfile"], + st.session_state.settings["coinmarketcap_token"], + ) if section == "Assets Balances": # For Assets Balances, always use local tokens since we need count data available_tokens = tokensdb.get_tokens() @@ -592,9 +599,9 @@ def build_tabs(section: str = "Assets Balances", use_api: bool = False): st.warning( "Unable to fetch tokens from API, falling back to local database" ) - available_tokens = markgetdb.get_tokens() + available_tokens = marketdb.get_tokens() else: - available_tokens = markgetdb.get_tokens() + available_tokens = marketdb.get_tokens() else: available_tokens = [] if not available_tokens: @@ -629,6 +636,8 @@ def build_tabs(section: str = "Assets Balances", use_api: bool = False): st.session_state.tokens[idx_token], start_timestamp, end_timestamp, + tokensdb, + marketdb, use_api, ) idx_token += 1 @@ -738,6 +747,7 @@ def build_price_tab( if st.button("Clear API Cache", help="Clear cached API responses"): fetch_api_coins.clear() fetch_api_crypto_market.clear() + fetch_api_fiat_rates.clear() st.success("Cache cleared!") st.rerun() else: @@ -753,12 +763,6 @@ def build_price_tab( key="currency_direction_toggle", ) -tokensdb = TokensDatabase(st.session_state.settings["dbfile"]) -markgetdb = Market( - st.session_state.settings["dbfile"], - st.session_state.settings["coinmarketcap_token"], -) - if "tokens" not in st.session_state: # Load saved token preferences from settings st.session_state.tokens = st.session_state.settings.get( diff --git a/app_pages/3_Operations.py b/app_pages/3_Operations.py index 46372c2..7cc2fdc 100644 --- a/app_pages/3_Operations.py +++ b/app_pages/3_Operations.py @@ -380,7 +380,7 @@ def buy_delete_dialog(data: dict): @st.dialog("Delete Swap") -def swap_delete_dialog(rows: list): +def swap_delete_dialog(rows: list, df_swaps_active: pd.DataFrame): """Display confirmation dialog for deleting a swap operation. Args: @@ -391,7 +391,7 @@ def swap_delete_dialog(rows: list): """ todelete = [] for rowidx in rows: - data = df_swap.iloc[rowidx].to_dict() + data = df_swaps_active.iloc[rowidx].to_dict() todelete.append(data) logger.debug("Dialog Delete row: %s", todelete) @@ -405,7 +405,7 @@ def swap_delete_dialog(rows: list): @st.dialog("Archive Swap") -def swap_archive_dialog(rows: list): +def swap_archive_dialog(rows: list, df_swaps_active: pd.DataFrame): """Display confirmation dialog for archiving a swap operation. Args: @@ -416,7 +416,7 @@ def swap_archive_dialog(rows: list): """ toarchive = [] for rowidx in rows: - data = df_swap.iloc[rowidx].to_dict() + data = df_swaps_active.iloc[rowidx].to_dict() toarchive.append(data) logger.debug("Dialog Archive row: %s", toarchive) @@ -430,7 +430,7 @@ def swap_archive_dialog(rows: list): @st.dialog("Unarchive Swap") -def swap_unarchive_dialog(rows: list): +def swap_unarchive_dialog(rows: list, df_swaps_archived: pd.DataFrame): """Display confirmation dialog for unarchiving a swap operation. Args: @@ -441,7 +441,7 @@ def swap_unarchive_dialog(rows: list): """ tounarchive = [] for rowidx in rows: - data = df_swap_arch.iloc[rowidx].to_dict() + data = df_swaps_archived.iloc[rowidx].to_dict() tounarchive.append(data) logger.debug("Dialog Unarchive row: %s", tounarchive) @@ -457,7 +457,7 @@ def swap_unarchive_dialog(rows: list): @st.dialog("Delete Archived Swap") -def swap_arch_delete_dialog(rows: list): +def swap_arch_delete_dialog(rows: list, df_swaps_archived: pd.DataFrame): """Display confirmation dialog for deleting an archived swap operation. Args: @@ -468,7 +468,7 @@ def swap_arch_delete_dialog(rows: list): """ todelete = [] for rowidx in rows: - data = df_swap_arch.iloc[rowidx].to_dict() + data = df_swaps_archived.iloc[rowidx].to_dict() todelete.append(data) logger.debug("Dialog Delete archived row: %s", todelete) @@ -512,7 +512,7 @@ def buy_delete(): buy_delete_dialog(df_buy.iloc[rowidx].to_dict()) -def swap_edit(): +def swap_edit(df_swaps_active: pd.DataFrame): """Handle editing of selected swap operation. Shows edit dialog if a row is selected. @@ -524,10 +524,10 @@ def swap_edit(): elif len(rowidx) > 1: st.toast("Please select only one row", icon=":material/warning:") else: - swap_edit_dialog(df_swap.iloc[rowidx[0]].to_dict()) + swap_edit_dialog(df_swaps_active.iloc[rowidx[0]].to_dict()) -def swap_delete(): +def swap_delete(df_swaps_active: pd.DataFrame): """Handle deletion of selected swap operation. Shows confirmation dialog if a row is selected. @@ -538,10 +538,10 @@ def swap_delete(): if rowidx is None: st.toast("Please select a row", icon=":material/warning:") else: - swap_delete_dialog(rowidx) + swap_delete_dialog(rowidx, df_swaps_active) -def swap_archive(): +def swap_archive(df_swaps_active: pd.DataFrame): """Handle archiving of selected swap operation. Shows confirmation dialog if a row is selected. @@ -552,10 +552,10 @@ def swap_archive(): if rowidx is None: st.toast("Please select a row", icon=":material/warning:") else: - swap_archive_dialog(rowidx) + swap_archive_dialog(rowidx, df_swaps_active) -def swap_unarchive(): +def swap_unarchive(df_swaps_archived: pd.DataFrame): """Handle unarchiving of selected swap operation. Shows confirmation dialog if a row is selected. @@ -566,10 +566,10 @@ def swap_unarchive(): if rowidx is None: st.toast("Please select a row", icon=":material/warning:") else: - swap_unarchive_dialog(rowidx) + swap_unarchive_dialog(rowidx, df_swaps_archived) -def swap_arch_delete(): +def swap_arch_delete(df_swaps_archived: pd.DataFrame): """Handle deletion of selected archived swap operation. Shows confirmation dialog if a row is selected. @@ -580,7 +580,7 @@ def swap_arch_delete(): if rowidx is None: st.toast("Please select a row", icon=":material/warning:") else: - swap_arch_delete_dialog(rowidx) + swap_arch_delete_dialog(rowidx, df_swaps_archived) def calc_perf(df: pd.DataFrame, col_token: str, col_rate: str) -> pd.DataFrame: @@ -812,14 +812,10 @@ def build_swap_dataframes( df1 = pd.DataFrame(swaps.get_by_tag(""), columns=columns) if not df1.empty: df1 = build_swap_dataframe(df1, db_file, convert_from, convert_to, use_api) - else: - st.info("No swap operations") df2 = pd.DataFrame(swaps.get_by_tag("archived"), columns=columns) if not df2.empty: df2 = build_swap_dataframe(df2, db_file, convert_from, convert_to, use_api) - else: - st.info("No archived swaps") return df1, df2 @@ -1205,7 +1201,7 @@ def color_avg_rows(row): swap_ct = None if swap_convert_to_sel == "Original" else swap_convert_to_sel # build swap table with performance metrics - df_swap, df_swap_arch = build_swap_dataframes( + df_swaps_active, df_swaps_archived = build_swap_dataframes( st.session_state.settings["dbfile"], convert_from=swap_cf, convert_to=swap_ct, @@ -1214,7 +1210,7 @@ def color_avg_rows(row): col_swaplist, col_swapbtns = st.columns([8, 1]) with col_swaplist: - draw_swap(df_swap, convert_from=swap_cf, convert_to=swap_ct) + draw_swap(df_swaps_active, convert_from=swap_cf, convert_to=swap_ct) with col_swapbtns: st.button( "New", @@ -1226,6 +1222,7 @@ def color_avg_rows(row): st.button( "Edit", on_click=swap_edit, + args=(df_swaps_active,), width="stretch", icon=":material/edit:", key="swap_edit", @@ -1233,6 +1230,7 @@ def color_avg_rows(row): st.button( "Archive", on_click=swap_archive, + args=(df_swaps_active,), width="stretch", icon=":material/archive:", key="swap_archive", @@ -1240,6 +1238,7 @@ def color_avg_rows(row): st.button( "Delete", on_click=swap_delete, + args=(df_swaps_active,), width="stretch", icon=":material/delete:", key="swap_delete", @@ -1248,11 +1247,12 @@ def color_avg_rows(row): st.title("Archived Swaps") col_archlist, col_archbtns = st.columns([8, 1]) with col_archlist: - draw_swap_arch(df_swap_arch, convert_from=swap_cf, convert_to=swap_ct) + draw_swap_arch(df_swaps_archived, convert_from=swap_cf, convert_to=swap_ct) with col_archbtns: st.button( "Unarchive", on_click=swap_unarchive, + args=(df_swaps_archived,), width="stretch", icon=":material/unarchive:", key="swap_unarchive", @@ -1260,6 +1260,7 @@ def color_avg_rows(row): st.button( "Delete", on_click=swap_arch_delete, + args=(df_swaps_archived,), width="stretch", icon=":material/delete:", key="swap_arch_delete", diff --git a/app_pages/4_Import.py b/app_pages/4_Import.py index 1bcf6e1..8b2a01f 100644 --- a/app_pages/4_Import.py +++ b/app_pages/4_Import.py @@ -69,12 +69,16 @@ def extract(input_data: any) -> pd.DataFrame: try: if isinstance(input_data, bytes): message_json, _ = aiprocessing.extract_from_img( - input_data, st.session_state.settings["ai_apitoken"] + input_data, + st.session_state.settings["ai_apitoken"], + model=st.session_state.settings.get("ai_model"), ) output = pd.DataFrame.from_dict(loads(message_json).get("assets")) elif isinstance(input_data, pd.DataFrame): message_json, _ = aiprocessing.extract_from_df( - input_data, st.session_state.settings["ai_apitoken"] + input_data, + st.session_state.settings["ai_apitoken"], + model=st.session_state.settings.get("ai_model"), ) output = pd.DataFrame.from_dict(loads(message_json).get("assets")) else: diff --git a/code-review-report.md b/code-review-report.md new file mode 100644 index 0000000..b76edd4 --- /dev/null +++ b/code-review-report.md @@ -0,0 +1,137 @@ +# Code Review Report - CryptoUpdate + +**Date**: 2026-04-18 +**Scope**: 26 Python files (app.py, app_pages, modules, modules/database, tests) + +--- + +## Findings by File + +### app_pages/1_Portfolios.py + +- � **L2** - `import traceback` at module top but only used in one `except` block; switch to `logger.exception()` and remove the import. +- 🟡 **L304-312** - `is_portfolio_empty()` calls `create_portfolio_dataframe(pf)`, which triggers full Market DB price lookups for every portfolio on every render just to determine emptiness. Extract a cheap amount-only check from raw portfolio data. +- 🔵 **L399** - `traceback.print_exc()` in bare `except Exception` block; replace with `logger.exception()`. + +### app_pages/2_Graphs.py + +- 🟡 **L167** - `st.plotly_chart(fig, width="stretch")` in `plot_modern_graph` — `width` is not a valid `st.plotly_chart` keyword; use `use_container_width=True`. +- 🟡 **L288** - Same issue in `plot_dual_axis_graph`. + +### app_pages/6_Settings.py + +- 🟡 **L33-52** - `_check_marketraccoon()` can return `"invalid_url"` (for `MissingSchema`), but the sidebar caller has no `elif _status == "invalid_url"` branch — the status is silently swallowed and the user sees nothing. +- 🔵 **L16-29** - Only specific `requests` exceptions are caught; other `RequestException` subclasses (e.g. `SSLError`, `TooManyRedirects`) propagate through `@st.cache_data` and crash the sidebar widget. + +### app_pages/X_Tests.py + +- 🔵 **L67** - `import requests` inside `check_api_data_for_date`; move to module top. +- 🟡 **L163** - `ThreadPoolExecutor(max_workers=10)` fires up to `len(missing_dates)` concurrent requests to MarketRaccoon with no rate limiting or backoff. Risk of 429 on large date gaps. +- 🔵 **L240** - `logger.error(...)` in the `as_completed` exception handler; use `logger.exception()` to capture the traceback. + +### modules/aiprocessing.py + +- 🔵 **L157** - `traceback.print_exc()` in `call_ai()` exception handler; replace with `logger.exception()` and remove `import traceback`. + +### modules/cmc.py + +- 🟡 **L11** - `__init__` annotated `-> dict`; should be `-> None`. +- 🔵 **L2** - `import traceback` only used in one error handler; replace with `logger.exception()` and remove the import. +- 🔵 **L78** - `traceback.print_exc()` in `get_current_fiat_prices()` error handler; replace with `logger.exception()`. + +### modules/configuration.py + +- 🔵 **L58, L62** - `os.path.basename(settings["dbfile"])` and `os.path.basename(settings["archive_path"])` silently discard any subdirectory component. A path like `data/subdir/db.sqlite3` would be saved as `db.sqlite3`, losing `subdir/` on the next read. + +### modules/token_metadata.py + +- 🟡 **L329-386** - `upsert_token_info_by_mr_id` has three-level branching (look up by `mraccoon_id` → look up by symbol → insert new). Correct but hard to follow; consider factoring the two lookup queries into named helpers. +- 🟡 **L388-401** - `delete_token(token)` issues `DELETE FROM TokenMetadata WHERE token = ?`. Since `token` is no longer the PRIMARY KEY (migration v4 added `id AUTOINCREMENT`), multiple rows may exist per symbol — all are deleted. Verify this is the intended semantics or add a delete-by-`id` variant. + +### modules/tools.py + +- 🟡 **L361** - `get_dataframe()` slices `df[["Token", "Market Price", "Coins in wallet", "Timestamp"]]` without first validating the columns exist. A missing column produces an unhelpful `KeyError`; add an explicit presence check with a clear error message. +- 🟡 **L696** - `_get_api_fiat_rate()` calls `api.get_fiat_latest_rate()` (uncached) instead of `api.get_fiat_latest_rate_cached()`, bypassing `FiatCacheManager` TTL logic and causing redundant network requests. +- 🔵 **L256** - `traceback.print_exc()` in `update_database()` error handler; replace with `logger.exception()`. +- 🔵 **L1017** - `traceback.print_exc()` in `update()` error handler; replace with `logger.exception()`. + +### modules/database/apimarket.py + +- 🔵 **L182** - `dt.astimezone(pd.Timestamp.now(tz="UTC").tz).isoformat()` constructs a `pd.Timestamp` just to extract its `.tz` attribute; use `datetime.timezone.utc` directly: `dt.astimezone(datetime.timezone.utc).isoformat()`. +- 🔵 **L373** - Same redundant pattern as L182. + +### modules/database/customdata.py + +- 🟡 **L40** - `get()` is annotated `-> str` but returns `None` or a raw sqlite3 row tuple `(value, type)`. Misleading annotation; callers must know the real contract by convention. + +### modules/database/fiat_cache.py + +- 🔵 **L270-273** - `if "temp_path" in locals()` in the `_save_cache` exception handler is fragile. Initialize `temp_path: str | None = None` before the `try` block and test `if temp_path is not None`. + +### modules/database/market.py + +- 🟡 **L391** - `get_token_lowhigh()` is annotated `-> pd.DataFrame` but returns `tuple[pd.DataFrame, pd.DataFrame]` (low, high). +- 🟡 **L414** - `get_currency_lowhigh()` has the same incorrect annotation. + +### modules/database/portfolios.py + +- 🟡 **L55** - `delete_portfolio()` deletes only the `Portfolios` row, not the associated `Portfolios_Tokens` rows. SQLite FK enforcement is off by default and no `ON DELETE CASCADE` is set, so orphan holdings rows accumulate. +- 🔵 **L107** - `str(amount)` in `set_token()` SQL binding is redundant; sqlite3 accepts Python `float` natively. +- 🔵 **L145** - `str(new_amount)` in `set_token_add()` SQL binding is likewise redundant. + +--- + +## Consolidated Recommendations (Sorted by Impact) + +| # | Priority | Action | File(s) | Benefit | +|---|---|---|---|---| +| 1 | 🟡 | Reconnect fiat-rate path to cached accessor in `_get_api_fiat_rate` | [modules/tools.py](modules/tools.py):696 | Consistent caching, fewer redundant API calls | +| 2 | 🟡 | Add cascade cleanup when deleting portfolios | [modules/database/portfolios.py](modules/database/portfolios.py):55 | Referential integrity, no orphan rows | +| 3 | 🟡 | Handle `"invalid_url"` status in settings sidebar | [app_pages/6_Settings.py](app_pages/6_Settings.py):33-52 | No silent failure on misconfigured URL | +| 4 | 🟡 | Decouple `is_portfolio_empty` from valuation / price queries | [app_pages/1_Portfolios.py](app_pages/1_Portfolios.py):304-312 | Avoid expensive hidden work on every render | +| 5 | 🟡 | Fix invalid `width="stretch"` in `st.plotly_chart()` calls | [app_pages/2_Graphs.py](app_pages/2_Graphs.py):167,288 | Correct Streamlit/Plotly layout | +| 6 | 🟡 | Fix incorrect return type annotations | [modules/database/market.py](modules/database/market.py):391,414 · [modules/database/customdata.py](modules/database/customdata.py):40 · [modules/cmc.py](modules/cmc.py):11 | Accurate static analysis, safer callers | +| 7 | 🟡 | Validate expected columns in `get_dataframe` before slicing | [modules/tools.py](modules/tools.py):361 | Early, descriptive failure instead of KeyError | +| 8 | 🟡 | Add rate limiting / backoff in threaded API date checks | [app_pages/X_Tests.py](app_pages/X_Tests.py):163 | Avoid 429 throttling from MarketRaccoon | +| 9 | 🟡 | Clarify `delete_token(token)` all-rows semantics or add delete-by-id variant | [modules/token_metadata.py](modules/token_metadata.py):388-401 | Safer data ops since token is no longer PK | +| 10 | 🟡 | Refactor `upsert_token_info_by_mr_id` three-branch logic into named helpers | [modules/token_metadata.py](modules/token_metadata.py):329-386 | Readability and maintainability | +| 11 | 🔵 | Replace all `traceback.print_exc()` with `logger.exception()` and drop `import traceback` | [modules/aiprocessing.py](modules/aiprocessing.py) · [modules/cmc.py](modules/cmc.py) · [modules/tools.py](modules/tools.py):256,1017 · [app_pages/1_Portfolios.py](app_pages/1_Portfolios.py):399 | Structured observability, clean imports | +| 12 | 🔵 | Catch `requests.exceptions.RequestException` broadly in `_check_marketraccoon` | [app_pages/6_Settings.py](app_pages/6_Settings.py):16-29 | Prevent unhandled SSL/proxy errors crashing sidebar | +| 13 | 🔵 | Fix `os.path.basename()` dropping subdirectory in `save_config` | [modules/configuration.py](modules/configuration.py):58,62 | Correct round-trip save for non-flat paths | +| 14 | 🔵 | Replace `if "temp_path" in locals()` with `temp_path = None` pre-init | [modules/database/fiat_cache.py](modules/database/fiat_cache.py):270-273 | Robust cleanup in all Python implementations | +| 15 | 🔵 | Simplify `pd.Timestamp.now(tz="UTC").tz` → `datetime.timezone.utc` | [modules/database/apimarket.py](modules/database/apimarket.py):182,373 | Cleaner, dependency-free UTC usage | +| 16 | 🔵 | Remove redundant `str(amount)` / `str(new_amount)` in SQL bindings | [modules/database/portfolios.py](modules/database/portfolios.py):107,145 | Idiomatic sqlite3 float binding | +| 17 | 🔵 | Move `import requests` to module top | [app_pages/X_Tests.py](app_pages/X_Tests.py):67 | PEP 8 / import hygiene | + +--- + +## Files with No Findings + +- [app.py](app.py) +- [tests/test_utils.py](tests/test_utils.py) +- [app_pages/0_Home.py](app_pages/0_Home.py) +- [app_pages/3_Operations.py](app_pages/3_Operations.py) +- [app_pages/4_Import.py](app_pages/4_Import.py) +- [app_pages/5_TokenMetadata.py](app_pages/5_TokenMetadata.py) +- [modules/plotter.py](modules/plotter.py) +- [modules/utils.py](modules/utils.py) +- [modules/database/migrations.py](modules/database/migrations.py) +- [modules/database/operations.py](modules/database/operations.py) +- [modules/database/swaps.py](modules/database/swaps.py) +- [modules/database/tokensdb.py](modules/database/tokensdb.py) + +--- + +## Executive Summary + +The codebase is in significantly improved shape compared to the previous review cycle. All previously reported critical and high-severity defects (outer-merge memory explosion, stale closure in dialogs, `st.info()` inside cached functions, O(n²) DataFrame append in `add_tokens`, invalid `width="stretch"` in plotter/home, `backup_database` with no disk check or rotation) have been resolved. + +The remaining 17 findings fall into three themes: + +1. **API caching consistency** (finding #1): `_get_api_fiat_rate()` bypasses `FiatCacheManager` by calling the raw uncached accessor — the highest-priority fix as it undermines the cache layer's TTL behaviour. + +2. **Type annotation accuracy** (findings #6, #7): Four methods (`customdata.get()`, both `lowhigh` methods in `market`, `cmc.__init__`) carry annotations that contradict actual return types, producing false-negative static-analysis results and requiring callers to know the real contract by convention. + +3. **`traceback.print_exc()` residue** (finding #11): Five locations across four files still use the stdlib function for error logging instead of the structured `logger.exception()` pattern adopted everywhere else. Each fix is a two-line change. + +Secondary concerns: the missing `"invalid_url"` branch in the settings sidebar (silent gap, finding #3), expensive valuation work hidden inside `is_portfolio_empty()` (finding #4), absent cascade delete for portfolio removal (finding #2), and the two `width="stretch"` instances still in `2_Graphs.py` (finding #5) which are direct copies of the pattern already corrected in `plotter.py` and `0_Home.py`. diff --git a/modules/aiprocessing.py b/modules/aiprocessing.py index 174ccdf..98afbf4 100644 --- a/modules/aiprocessing.py +++ b/modules/aiprocessing.py @@ -16,7 +16,21 @@ logger = logging.getLogger(__name__) -CLAUDE_MODEL = "claude-sonnet-4-20250514" +# Default model — can be overridden by settings.json key "AI.model" +_DEFAULT_CLAUDE_MODEL = "claude-sonnet-4-20250514" + +# Module-level singleton; rebuilt only when the API key changes. +_anthropic_client: Anthropic | None = None +_anthropic_client_key: str | None = None + + +def _get_client(api_key: str) -> Anthropic: + """Return a cached Anthropic client, rebuilding only when the key changes.""" + global _anthropic_client, _anthropic_client_key + if _anthropic_client is None or _anthropic_client_key != api_key: + _anthropic_client = Anthropic(api_key=api_key) + _anthropic_client_key = api_key + return _anthropic_client def get_image_type(image: bytes): @@ -29,7 +43,7 @@ def get_image_type(image: bytes): return None -def extract_from_df(df: pd.DataFrame, api_key: str): +def extract_from_df(df: pd.DataFrame, api_key: str, model: str | None = None): system_prompt = ( "You are a data extraction model. You must return ONLY valid JSON, " "with no markdown formatting, no code blocks, and no explanatory text. " @@ -69,10 +83,10 @@ def extract_from_df(df: pd.DataFrame, api_key: str): ), }, ] - return call_ai(messages, api_key, system_prompt) + return call_ai(messages, api_key, system_prompt, model=model) -def extract_from_img(bytes_data: bytes, api_key: str): +def extract_from_img(bytes_data: bytes, api_key: str, model: str | None = None): type_image = get_image_type(bytes_data) if type_image is None: logger.debug("Invalid image.") @@ -133,18 +147,18 @@ def extract_from_img(bytes_data: bytes, api_key: str): ], }, ] - return call_ai(messages, api_key, system_prompt) + return call_ai(messages, api_key, system_prompt, model=model) -def call_ai(messages: list, api_key: str, system_prompt: str = ""): - # Create an Anthropic client - client = Anthropic(api_key=api_key) +def call_ai(messages: list, api_key: str, system_prompt: str = "", model: str | None = None): + client = _get_client(api_key) + claude_model = model or _DEFAULT_CLAUDE_MODEL total_tokens = 0 try: logger.debug("Processing ...") response = client.messages.create( - model=CLAUDE_MODEL, + model=claude_model, max_tokens=4096, system=system_prompt, messages=messages, diff --git a/modules/database/portfolios.py b/modules/database/portfolios.py index 6349f27..529beab 100644 --- a/modules/database/portfolios.py +++ b/modules/database/portfolios.py @@ -11,7 +11,6 @@ logging.getLogger("matplotlib").setLevel(logging.WARNING) logging.getLogger("PIL").setLevel(logging.WARNING) -logging.getLogger("modules.process").setLevel(logging.WARNING) logger = logging.getLogger(__name__) @@ -140,11 +139,11 @@ def set_token_add(self, name: str, token: str, amount: float): ? ) """, - (name, token, str(amount)), + (name, token, amount), ) conn.commit() - def delete_token_a(self, name: str, token: str): + def delete_token_by_name(self, name: str, token: str): with sqlite3.connect(self.db_path) as conn: cursor = conn.cursor() cursor.execute( @@ -156,7 +155,7 @@ def delete_token_a(self, name: str, token: str): ) conn.commit() - def delete_token_b(self, portfolio_id: int, token: str): + def delete_token_by_id(self, portfolio_id: int, token: str): with sqlite3.connect(self.db_path) as conn: cursor = conn.cursor() cursor.execute( diff --git a/modules/database/swaps.py b/modules/database/swaps.py index ef13f9a..ce925f7 100644 --- a/modules/database/swaps.py +++ b/modules/database/swaps.py @@ -8,7 +8,6 @@ import logging import sqlite3 -import traceback logger = logging.getLogger(__name__) @@ -115,8 +114,7 @@ def delete(self, entry_id: int): conn.commit() logger.debug(f"Entry with id {entry_id} deleted successfully.") except Exception as e: - logger.error(f"Error deleting swap: {e}") - traceback.print_exc() + logger.exception("Error deleting swap: %s", e) def update_note(self, entry_id: int, note: str): logger.debug("Updating swap note") @@ -128,8 +126,7 @@ def update_note(self, entry_id: int, note: str): conn.commit() logger.debug(f"Note updated for entry with id {entry_id}") except Exception as e: - logger.error(f"Error updating note: {e}") - traceback.print_exc() + logger.exception("Error updating note: %s", e) def update_tag(self, entry_id: int, tag: str): logger.debug("Updating swap tag") @@ -147,5 +144,4 @@ def update_tag(self, entry_id: int, tag: str): conn.commit() logger.debug(f"Tag updated for entry with id {entry_id}") except Exception as e: - logger.error(f"Error updating tag: {e}") - traceback.print_exc() + logger.exception("Error updating tag: %s", e) diff --git a/modules/database/tokensdb.py b/modules/database/tokensdb.py index b46e54d..f1d1582 100644 --- a/modules/database/tokensdb.py +++ b/modules/database/tokensdb.py @@ -183,19 +183,16 @@ def add_tokens(self, tokens: dict): logger.debug("Adding data to database:\n%s", tokens) timestamp = int(pd.Timestamp.now(tz="UTC").timestamp()) - df: pd.DataFrame = pd.DataFrame( - columns=["timestamp", "token", "price", "count"] - ) - for token, data in tokens.items(): - if "timestamp" in data: - df.loc[len(df)] = [ - data["timestamp"], - token, - data["price"], - data["amount"], - ] - else: - df.loc[len(df)] = [timestamp, token, data["price"], data["amount"]] + rows = [ + { + "timestamp": data.get("timestamp", timestamp), + "token": token, + "price": data["price"], + "count": data["amount"], + } + for token, data in tokens.items() + ] + df = pd.DataFrame(rows, columns=["timestamp", "token", "price", "count"]) logger.debug("Dataframe to add:\n%s", df) with sqlite3.connect(self.db_path) as con: df.to_sql("TokensDatabase", con, if_exists="append", index=False) diff --git a/modules/plotter.py b/modules/plotter.py index adbb7e8..3821311 100644 --- a/modules/plotter.py +++ b/modules/plotter.py @@ -14,7 +14,7 @@ def plot_as_pie(df: pd.DataFrame, column): return total = df[column].sum() - limit = (1 * total) / 100 + limit = total / 100 logger.debug(f"1% of {total} is {limit}") # Group token representing less then 1% of total value @@ -33,7 +33,7 @@ def plot_as_pie(df: pd.DataFrame, column): logger.debug(f"Dataframe more than 1% sum with Others:\n{dffinal}") fig = px.pie(dffinal, dffinal.index, column, width=700, height=700) - st.plotly_chart(fig, width="stretch") + st.plotly_chart(fig, use_container_width=True) def plot_as_graph(df: pd.DataFrame): diff --git a/modules/tools.py b/modules/tools.py index 404c787..48e6a90 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -8,6 +8,7 @@ """ from datetime import datetime +import glob import logging import os import shutil @@ -987,24 +988,47 @@ def update(): traceback.print_exc() -def backup_database(dbfile: str) -> str: +def backup_database(dbfile: str, max_backups: int = 10) -> str: """Crée une sauvegarde du fichier de base de données en ajoutant un timestamp dans le nom. Args: dbfile: Chemin vers le fichier de base de données + max_backups: Nombre maximum de sauvegardes à conserver (défaut: 10) Returns: Chemin vers le fichier de sauvegarde créé Raises: FileNotFoundError: Si le fichier source n'existe pas + OSError: Si l'espace disque est insuffisant """ if not os.path.exists(dbfile): raise FileNotFoundError(f"Fichier de base de données introuvable : {dbfile}") + db_size = os.path.getsize(dbfile) + disk_usage = shutil.disk_usage(os.path.dirname(os.path.abspath(dbfile))) + if disk_usage.free < db_size * 2: + raise OSError( + f"Espace disque insuffisant pour la sauvegarde : " + f"{disk_usage.free // (1024**2)} Mo disponibles, " + f"{db_size * 2 // (1024**2)} Mo requis" + ) + timestamp = datetime.now().strftime("%Y%m%d_%H%M%S") backup_file = f"{dbfile}_{timestamp}.bak" shutil.copy2(dbfile, backup_file) logger.info("Base de données sauvegardée dans : %s", backup_file) + + # Rotation : conserver uniquement les max_backups sauvegardes les plus récentes + pattern = f"{dbfile}_*.bak" + existing_backups = sorted(glob.glob(pattern)) + while len(existing_backups) > max_backups: + oldest = existing_backups.pop(0) + try: + os.remove(oldest) + logger.info("Ancienne sauvegarde supprimée : %s", oldest) + except OSError as e: + logger.warning("Impossible de supprimer l'ancienne sauvegarde %s : %s", oldest, e) + return backup_file diff --git a/modules/utils.py b/modules/utils.py index 24bb568..759c69f 100644 --- a/modules/utils.py +++ b/modules/utils.py @@ -124,7 +124,65 @@ def debug_prefix(input_str: str, flag=False) -> str: def dataframe_diff(df1, df2): """Find rows that are different between two DataFrames""" - comparison_df = df1.merge(df2, indicator=True, how="outer") + left_df = df1.copy() + right_df = df2.copy() + + if list(left_df.columns) != list(right_df.columns): + all_columns = sorted(set(left_df.columns).union(right_df.columns)) + left_df = left_df.reindex(columns=all_columns) + right_df = right_df.reindex(columns=all_columns) + + left_df["_source"] = "left" + right_df["_source"] = "right" + combined_df = pd.concat([left_df, right_df], ignore_index=True) + + row_columns = [col for col in combined_df.columns if col != "_source"] + combined_df["_count"] = 1 + + # Compare row multiplicities by source without using outer merge. + comparison_df = ( + combined_df.pivot_table( + index=row_columns, + columns="_source", + values="_count", + aggfunc="sum", + fill_value=0, + ) + .reset_index() + .rename_axis(columns=None) + ) + + if "left" not in comparison_df.columns: + comparison_df["left"] = 0 + if "right" not in comparison_df.columns: + comparison_df["right"] = 0 + + comparison_df["left"] = comparison_df["left"].astype(int) + comparison_df["right"] = comparison_df["right"].astype(int) + + left_only_repeats = (comparison_df["left"] - comparison_df["right"]).clip(lower=0) + right_only_repeats = (comparison_df["right"] - comparison_df["left"]).clip(lower=0) + + diff_parts = [] + + if left_only_repeats.sum() > 0: + left_only_df = comparison_df.loc[ + comparison_df.index.repeat(left_only_repeats), row_columns + ].copy() + left_only_df["_merge"] = "left_only" + diff_parts.append(left_only_df) + + if right_only_repeats.sum() > 0: + right_only_df = comparison_df.loc[ + comparison_df.index.repeat(right_only_repeats), row_columns + ].copy() + right_only_df["_merge"] = "right_only" + diff_parts.append(right_only_df) + + if not diff_parts: + return pd.DataFrame(columns=row_columns + ["_merge"]) + + diff_df = pd.concat(diff_parts, ignore_index=True) logger.debug("comparison_df:\n%s", comparison_df) - diff_df = comparison_df[comparison_df["_merge"] != "both"] + logger.debug("diff_df:\n%s", diff_df) return diff_df From 6edef8c2e4adcf0023e98539ba8f9546aa555129 Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Sat, 18 Apr 2026 13:09:17 +0300 Subject: [PATCH 11/13] refactor: complete code review fixes across all modules - Replace remaining traceback.print_exc() with logger.exception() - Fix f-string logging to use %s format throughout - Add migration v7: convert Swaps amount columns from TEXT to REAL - Wire configurable ratesdb_url through settings, config, and Market - Fix fiat rate caching bypass in _get_api_fiat_rate - Harden currency update: dedup dates, retry on transient errors, batch insert - Fix plotly use_container_width, path preservation in save_config, and misc bugs --- app_pages/0_Home.py | 2 +- app_pages/1_Portfolios.py | 11 +-- app_pages/2_Graphs.py | 8 +- app_pages/3_Operations.py | 2 +- app_pages/4_Import.py | 8 +- app_pages/5_TokenMetadata.py | 2 +- app_pages/6_Settings.py | 18 +++++ app_pages/X_Tests.py | 9 +-- code-review-report.md | 137 ------------------------------- modules/aiprocessing.py | 5 +- modules/cmc.py | 17 ++-- modules/configuration.py | 40 +++++++-- modules/database/apimarket.py | 10 +-- modules/database/fiat_cache.py | 48 +++++------ modules/database/market.py | 143 +++++++++++++++++++++++---------- modules/database/migrations.py | 107 ++++++++++++++---------- modules/database/portfolios.py | 8 +- modules/database/swaps.py | 10 +-- modules/database/tokensdb.py | 7 +- modules/plotter.py | 12 +-- modules/token_metadata.py | 2 +- modules/tools.py | 28 ++++--- modules/utils.py | 4 +- 23 files changed, 318 insertions(+), 320 deletions(-) delete mode 100644 code-review-report.md diff --git a/app_pages/0_Home.py b/app_pages/0_Home.py index b7c65a9..1dcf0df 100644 --- a/app_pages/0_Home.py +++ b/app_pages/0_Home.py @@ -96,7 +96,7 @@ def plot_total_value(df: pd.DataFrame): balance = ( 0 if df_balance is None or df_balance.empty - else df_balance.iloc[-1, 1:].sum() + else df_balance.iloc[-1].sum() ) balance = round(balance, 2) st.metric("Total value", value=f"{balance} {currency_symbol}") diff --git a/app_pages/1_Portfolios.py b/app_pages/1_Portfolios.py index a2003bd..06d4ce1 100644 --- a/app_pages/1_Portfolios.py +++ b/app_pages/1_Portfolios.py @@ -1,5 +1,4 @@ import logging -import traceback import pandas as pd import streamlit as st @@ -303,13 +302,9 @@ def save_toggle_preference(): def is_portfolio_empty(pf_name): pf = g_portfolios.get_portfolio(pf_name) - df = create_portfolio_dataframe(pf) - if df.empty: + if not pf: return True - # Si tous les montants sont nuls ou absents - if (df["amount"].fillna(0) == 0).all(): - return True - return False + return all((v.get("amount") or 0) == 0 for v in pf.values()) all_tabs = g_portfolios.get_portfolio_names() @@ -328,4 +323,4 @@ def is_portfolio_empty(pf_name): portfolio_ui(tabs) except Exception as e: st.error(f"UI Error: {str(e)}") - traceback.print_exc() + logger.exception("Portfolio UI rendering failed") diff --git a/app_pages/2_Graphs.py b/app_pages/2_Graphs.py index c879632..d884767 100644 --- a/app_pages/2_Graphs.py +++ b/app_pages/2_Graphs.py @@ -159,7 +159,7 @@ def plot_modern_graph( fig.update_layout(**layout_config) - st.plotly_chart(fig, width="stretch") + st.plotly_chart(fig, use_container_width=True) def plot_dual_axis_graph(df: pd.DataFrame, title: str = None, token: str = None): @@ -257,7 +257,7 @@ def plot_dual_axis_graph(df: pd.DataFrame, title: str = None, token: str = None) legend=dict(orientation="h", yanchor="bottom", y=1.02, xanchor="right", x=1), ) - st.plotly_chart(fig, width="stretch") + st.plotly_chart(fig, use_container_width=True) def load_portfolios(dbfile: str) -> Portfolios: @@ -651,9 +651,9 @@ def build_price_tab( st.info("No data available") return if st.session_state.startdate < st.session_state.enddate: - df_view = df.loc[df.index > str(st.session_state.startdate)] + df_view = df.loc[df.index > pd.Timestamp(st.session_state.startdate)] df_view = df_view.loc[ - df_view.index < str(st.session_state.enddate + pd.to_timedelta(1, unit="d")) + df_view.index < pd.Timestamp(st.session_state.enddate) + pd.to_timedelta(1, unit="d") ] df_view = df_view.loc[:, ["price"]] df_view = df_view.dropna() diff --git a/app_pages/3_Operations.py b/app_pages/3_Operations.py index 7cc2fdc..ea07e40 100644 --- a/app_pages/3_Operations.py +++ b/app_pages/3_Operations.py @@ -788,7 +788,7 @@ def build_buy_avg_table(use_api: bool = False): @st.cache_data( - hash_funcs={str: lambda x: get_file_hash(x) if os.path.isfile(x) else hash(x)}, + hash_funcs={str: lambda x: get_file_hash(x) if (os.path.sep in x and os.path.isfile(x)) else hash(x)}, ) def build_swap_dataframes( db_file: str, diff --git a/app_pages/4_Import.py b/app_pages/4_Import.py index 8b2a01f..dcb8629 100644 --- a/app_pages/4_Import.py +++ b/app_pages/4_Import.py @@ -73,14 +73,14 @@ def extract(input_data: any) -> pd.DataFrame: st.session_state.settings["ai_apitoken"], model=st.session_state.settings.get("ai_model"), ) - output = pd.DataFrame.from_dict(loads(message_json).get("assets")) + output = pd.DataFrame(loads(message_json).get("assets")) elif isinstance(input_data, pd.DataFrame): message_json, _ = aiprocessing.extract_from_df( input_data, st.session_state.settings["ai_apitoken"], model=st.session_state.settings.get("ai_model"), ) - output = pd.DataFrame.from_dict(loads(message_json).get("assets")) + output = pd.DataFrame(loads(message_json).get("assets")) else: raise ValueError("Invalid input type") except (ValueError, KeyError, TypeError) as e: @@ -155,7 +155,7 @@ def draw_ui(): """ col_input, col_output = st.columns(2) with col_input: - if st.session_state.import_page["type"] == "application/vnd.ms-excel": + if isinstance(st.session_state.import_page["input"], pd.DataFrame): st.dataframe( st.session_state.import_page["input"], column_config={ @@ -223,7 +223,7 @@ def clean_session_state(): else: logger.debug("File: %s - file type: %s", file.name, file.type) - if file.type in ("text/csv", "application/vnd.ms-excel") or file.name.endswith(".csv"): + if file.type == "text/csv" or file.name.endswith(".csv"): logger.debug("CSV file detected") input_file = process_csv(file) else: diff --git a/app_pages/5_TokenMetadata.py b/app_pages/5_TokenMetadata.py index 21d7489..b37b4b4 100644 --- a/app_pages/5_TokenMetadata.py +++ b/app_pages/5_TokenMetadata.py @@ -303,7 +303,7 @@ def _clear_checked_state(): _clear_checked_state() st.rerun() except Exception as e: - logger.exception(f"Error saving token metadata: {e}") + logger.exception("Error saving token metadata: %s", e) st.error(f"Error saving metadata: {e}") st.divider() diff --git a/app_pages/6_Settings.py b/app_pages/6_Settings.py index d02cfa3..fc79b65 100644 --- a/app_pages/6_Settings.py +++ b/app_pages/6_Settings.py @@ -25,6 +25,8 @@ def _check_marketraccoon(url: str, api_key: str) -> str: return "timeout" except requests.exceptions.MissingSchema: return "invalid_url" + except requests.exceptions.RequestException: + return "request_error" with st.sidebar: @@ -43,6 +45,12 @@ def _check_marketraccoon(url: str, api_key: str) -> str: elif _status == "timeout": st.error("MarketRaccoon :material/timer_off:") logger.error("API request timed out.") + elif _status == "invalid_url": + st.error("MarketRaccoon :material/error:") + logger.error("Invalid MarketRaccoon URL format.") + elif _status == "request_error": + st.error("MarketRaccoon :material/warning:") + logger.error("Unhandled request error during API healthcheck.") with st.form(key="settings_form"): st.subheader("MarketRaccoon") @@ -58,6 +66,15 @@ def _check_marketraccoon(url: str, api_key: str) -> str: value=st.session_state.settings.get("marketraccoon_token", ""), ) + ratesdb_url = st.text_input( + "RatesDB URL", + key="ratesdb_url", + value=st.session_state.settings.get( + "ratesdb_url", "https://free.ratesdb.com/v1/rates" + ), + help="Fallback API URL for historical EUR/USD rates", + ) + st.subheader("Coinmarketcap") coinmarketcap_token = st.text_input( "Coinmarketcap API token", @@ -140,6 +157,7 @@ def _check_marketraccoon(url: str, api_key: str) -> str: logger.debug("Submitted") st.session_state.settings["marketraccoon_url"] = marketraccoon_url st.session_state.settings["marketraccoon_token"] = marketraccoon_token + st.session_state.settings["ratesdb_url"] = ratesdb_url st.session_state.settings["coinmarketcap_token"] = coinmarketcap_token st.session_state.settings["ai_apitoken"] = ai_apitoken st.session_state.settings["debug_flag"] = debug_flag diff --git a/app_pages/X_Tests.py b/app_pages/X_Tests.py index afccea9..7d0f253 100644 --- a/app_pages/X_Tests.py +++ b/app_pages/X_Tests.py @@ -4,6 +4,7 @@ from datetime import datetime import pandas as pd +import requests import streamlit as st @@ -64,8 +65,6 @@ def check_api_data_for_date(api_url: str, api_key: str, date) -> bool: True if API has at least one cryptocurrency data point for that day, False otherwise """ try: - import requests - # Create start and end of day in ISO format start_dt = datetime.combine(date, datetime.min.time()) end_dt = datetime.combine(date, datetime.max.time()) @@ -92,7 +91,7 @@ def check_api_data_for_date(api_url: str, api_key: str, date) -> bool: results = data.get("results", []) # If we have at least one result for this day, return True - if results and len(results) > 0: + if results: logger.debug( "Found %d cryptocurrency data points for date %s", len(results), @@ -160,7 +159,7 @@ def check_api_data_for_date(api_url: str, api_key: str, date) -> bool: status_text = st.empty() # Use ThreadPoolExecutor to make 10 requests in parallel - with ThreadPoolExecutor(max_workers=10) as executor: + with ThreadPoolExecutor(max_workers=3) as executor: # Submit all tasks future_to_date = { executor.submit(check_api_data_for_date, api_url, api_key, date): date @@ -175,7 +174,7 @@ def check_api_data_for_date(api_url: str, api_key: str, date) -> bool: has_data = future.result() api_status_dict[date] = "✅" if has_data else "❌" except Exception as e: - logger.error("Error checking date %s: %s", date, str(e)) + logger.exception("Error checking date %s", date) api_status_dict[date] = "❌" completed += 1 diff --git a/code-review-report.md b/code-review-report.md deleted file mode 100644 index b76edd4..0000000 --- a/code-review-report.md +++ /dev/null @@ -1,137 +0,0 @@ -# Code Review Report - CryptoUpdate - -**Date**: 2026-04-18 -**Scope**: 26 Python files (app.py, app_pages, modules, modules/database, tests) - ---- - -## Findings by File - -### app_pages/1_Portfolios.py - -- � **L2** - `import traceback` at module top but only used in one `except` block; switch to `logger.exception()` and remove the import. -- 🟡 **L304-312** - `is_portfolio_empty()` calls `create_portfolio_dataframe(pf)`, which triggers full Market DB price lookups for every portfolio on every render just to determine emptiness. Extract a cheap amount-only check from raw portfolio data. -- 🔵 **L399** - `traceback.print_exc()` in bare `except Exception` block; replace with `logger.exception()`. - -### app_pages/2_Graphs.py - -- 🟡 **L167** - `st.plotly_chart(fig, width="stretch")` in `plot_modern_graph` — `width` is not a valid `st.plotly_chart` keyword; use `use_container_width=True`. -- 🟡 **L288** - Same issue in `plot_dual_axis_graph`. - -### app_pages/6_Settings.py - -- 🟡 **L33-52** - `_check_marketraccoon()` can return `"invalid_url"` (for `MissingSchema`), but the sidebar caller has no `elif _status == "invalid_url"` branch — the status is silently swallowed and the user sees nothing. -- 🔵 **L16-29** - Only specific `requests` exceptions are caught; other `RequestException` subclasses (e.g. `SSLError`, `TooManyRedirects`) propagate through `@st.cache_data` and crash the sidebar widget. - -### app_pages/X_Tests.py - -- 🔵 **L67** - `import requests` inside `check_api_data_for_date`; move to module top. -- 🟡 **L163** - `ThreadPoolExecutor(max_workers=10)` fires up to `len(missing_dates)` concurrent requests to MarketRaccoon with no rate limiting or backoff. Risk of 429 on large date gaps. -- 🔵 **L240** - `logger.error(...)` in the `as_completed` exception handler; use `logger.exception()` to capture the traceback. - -### modules/aiprocessing.py - -- 🔵 **L157** - `traceback.print_exc()` in `call_ai()` exception handler; replace with `logger.exception()` and remove `import traceback`. - -### modules/cmc.py - -- 🟡 **L11** - `__init__` annotated `-> dict`; should be `-> None`. -- 🔵 **L2** - `import traceback` only used in one error handler; replace with `logger.exception()` and remove the import. -- 🔵 **L78** - `traceback.print_exc()` in `get_current_fiat_prices()` error handler; replace with `logger.exception()`. - -### modules/configuration.py - -- 🔵 **L58, L62** - `os.path.basename(settings["dbfile"])` and `os.path.basename(settings["archive_path"])` silently discard any subdirectory component. A path like `data/subdir/db.sqlite3` would be saved as `db.sqlite3`, losing `subdir/` on the next read. - -### modules/token_metadata.py - -- 🟡 **L329-386** - `upsert_token_info_by_mr_id` has three-level branching (look up by `mraccoon_id` → look up by symbol → insert new). Correct but hard to follow; consider factoring the two lookup queries into named helpers. -- 🟡 **L388-401** - `delete_token(token)` issues `DELETE FROM TokenMetadata WHERE token = ?`. Since `token` is no longer the PRIMARY KEY (migration v4 added `id AUTOINCREMENT`), multiple rows may exist per symbol — all are deleted. Verify this is the intended semantics or add a delete-by-`id` variant. - -### modules/tools.py - -- 🟡 **L361** - `get_dataframe()` slices `df[["Token", "Market Price", "Coins in wallet", "Timestamp"]]` without first validating the columns exist. A missing column produces an unhelpful `KeyError`; add an explicit presence check with a clear error message. -- 🟡 **L696** - `_get_api_fiat_rate()` calls `api.get_fiat_latest_rate()` (uncached) instead of `api.get_fiat_latest_rate_cached()`, bypassing `FiatCacheManager` TTL logic and causing redundant network requests. -- 🔵 **L256** - `traceback.print_exc()` in `update_database()` error handler; replace with `logger.exception()`. -- 🔵 **L1017** - `traceback.print_exc()` in `update()` error handler; replace with `logger.exception()`. - -### modules/database/apimarket.py - -- 🔵 **L182** - `dt.astimezone(pd.Timestamp.now(tz="UTC").tz).isoformat()` constructs a `pd.Timestamp` just to extract its `.tz` attribute; use `datetime.timezone.utc` directly: `dt.astimezone(datetime.timezone.utc).isoformat()`. -- 🔵 **L373** - Same redundant pattern as L182. - -### modules/database/customdata.py - -- 🟡 **L40** - `get()` is annotated `-> str` but returns `None` or a raw sqlite3 row tuple `(value, type)`. Misleading annotation; callers must know the real contract by convention. - -### modules/database/fiat_cache.py - -- 🔵 **L270-273** - `if "temp_path" in locals()` in the `_save_cache` exception handler is fragile. Initialize `temp_path: str | None = None` before the `try` block and test `if temp_path is not None`. - -### modules/database/market.py - -- 🟡 **L391** - `get_token_lowhigh()` is annotated `-> pd.DataFrame` but returns `tuple[pd.DataFrame, pd.DataFrame]` (low, high). -- 🟡 **L414** - `get_currency_lowhigh()` has the same incorrect annotation. - -### modules/database/portfolios.py - -- 🟡 **L55** - `delete_portfolio()` deletes only the `Portfolios` row, not the associated `Portfolios_Tokens` rows. SQLite FK enforcement is off by default and no `ON DELETE CASCADE` is set, so orphan holdings rows accumulate. -- 🔵 **L107** - `str(amount)` in `set_token()` SQL binding is redundant; sqlite3 accepts Python `float` natively. -- 🔵 **L145** - `str(new_amount)` in `set_token_add()` SQL binding is likewise redundant. - ---- - -## Consolidated Recommendations (Sorted by Impact) - -| # | Priority | Action | File(s) | Benefit | -|---|---|---|---|---| -| 1 | 🟡 | Reconnect fiat-rate path to cached accessor in `_get_api_fiat_rate` | [modules/tools.py](modules/tools.py):696 | Consistent caching, fewer redundant API calls | -| 2 | 🟡 | Add cascade cleanup when deleting portfolios | [modules/database/portfolios.py](modules/database/portfolios.py):55 | Referential integrity, no orphan rows | -| 3 | 🟡 | Handle `"invalid_url"` status in settings sidebar | [app_pages/6_Settings.py](app_pages/6_Settings.py):33-52 | No silent failure on misconfigured URL | -| 4 | 🟡 | Decouple `is_portfolio_empty` from valuation / price queries | [app_pages/1_Portfolios.py](app_pages/1_Portfolios.py):304-312 | Avoid expensive hidden work on every render | -| 5 | 🟡 | Fix invalid `width="stretch"` in `st.plotly_chart()` calls | [app_pages/2_Graphs.py](app_pages/2_Graphs.py):167,288 | Correct Streamlit/Plotly layout | -| 6 | 🟡 | Fix incorrect return type annotations | [modules/database/market.py](modules/database/market.py):391,414 · [modules/database/customdata.py](modules/database/customdata.py):40 · [modules/cmc.py](modules/cmc.py):11 | Accurate static analysis, safer callers | -| 7 | 🟡 | Validate expected columns in `get_dataframe` before slicing | [modules/tools.py](modules/tools.py):361 | Early, descriptive failure instead of KeyError | -| 8 | 🟡 | Add rate limiting / backoff in threaded API date checks | [app_pages/X_Tests.py](app_pages/X_Tests.py):163 | Avoid 429 throttling from MarketRaccoon | -| 9 | 🟡 | Clarify `delete_token(token)` all-rows semantics or add delete-by-id variant | [modules/token_metadata.py](modules/token_metadata.py):388-401 | Safer data ops since token is no longer PK | -| 10 | 🟡 | Refactor `upsert_token_info_by_mr_id` three-branch logic into named helpers | [modules/token_metadata.py](modules/token_metadata.py):329-386 | Readability and maintainability | -| 11 | 🔵 | Replace all `traceback.print_exc()` with `logger.exception()` and drop `import traceback` | [modules/aiprocessing.py](modules/aiprocessing.py) · [modules/cmc.py](modules/cmc.py) · [modules/tools.py](modules/tools.py):256,1017 · [app_pages/1_Portfolios.py](app_pages/1_Portfolios.py):399 | Structured observability, clean imports | -| 12 | 🔵 | Catch `requests.exceptions.RequestException` broadly in `_check_marketraccoon` | [app_pages/6_Settings.py](app_pages/6_Settings.py):16-29 | Prevent unhandled SSL/proxy errors crashing sidebar | -| 13 | 🔵 | Fix `os.path.basename()` dropping subdirectory in `save_config` | [modules/configuration.py](modules/configuration.py):58,62 | Correct round-trip save for non-flat paths | -| 14 | 🔵 | Replace `if "temp_path" in locals()` with `temp_path = None` pre-init | [modules/database/fiat_cache.py](modules/database/fiat_cache.py):270-273 | Robust cleanup in all Python implementations | -| 15 | 🔵 | Simplify `pd.Timestamp.now(tz="UTC").tz` → `datetime.timezone.utc` | [modules/database/apimarket.py](modules/database/apimarket.py):182,373 | Cleaner, dependency-free UTC usage | -| 16 | 🔵 | Remove redundant `str(amount)` / `str(new_amount)` in SQL bindings | [modules/database/portfolios.py](modules/database/portfolios.py):107,145 | Idiomatic sqlite3 float binding | -| 17 | 🔵 | Move `import requests` to module top | [app_pages/X_Tests.py](app_pages/X_Tests.py):67 | PEP 8 / import hygiene | - ---- - -## Files with No Findings - -- [app.py](app.py) -- [tests/test_utils.py](tests/test_utils.py) -- [app_pages/0_Home.py](app_pages/0_Home.py) -- [app_pages/3_Operations.py](app_pages/3_Operations.py) -- [app_pages/4_Import.py](app_pages/4_Import.py) -- [app_pages/5_TokenMetadata.py](app_pages/5_TokenMetadata.py) -- [modules/plotter.py](modules/plotter.py) -- [modules/utils.py](modules/utils.py) -- [modules/database/migrations.py](modules/database/migrations.py) -- [modules/database/operations.py](modules/database/operations.py) -- [modules/database/swaps.py](modules/database/swaps.py) -- [modules/database/tokensdb.py](modules/database/tokensdb.py) - ---- - -## Executive Summary - -The codebase is in significantly improved shape compared to the previous review cycle. All previously reported critical and high-severity defects (outer-merge memory explosion, stale closure in dialogs, `st.info()` inside cached functions, O(n²) DataFrame append in `add_tokens`, invalid `width="stretch"` in plotter/home, `backup_database` with no disk check or rotation) have been resolved. - -The remaining 17 findings fall into three themes: - -1. **API caching consistency** (finding #1): `_get_api_fiat_rate()` bypasses `FiatCacheManager` by calling the raw uncached accessor — the highest-priority fix as it undermines the cache layer's TTL behaviour. - -2. **Type annotation accuracy** (findings #6, #7): Four methods (`customdata.get()`, both `lowhigh` methods in `market`, `cmc.__init__`) carry annotations that contradict actual return types, producing false-negative static-analysis results and requiring callers to know the real contract by convention. - -3. **`traceback.print_exc()` residue** (finding #11): Five locations across four files still use the stdlib function for error logging instead of the structured `logger.exception()` pattern adopted everywhere else. Each fix is a two-line change. - -Secondary concerns: the missing `"invalid_url"` branch in the settings sidebar (silent gap, finding #3), expensive valuation work hidden inside `is_portfolio_empty()` (finding #4), absent cascade delete for portfolio removal (finding #2), and the two `width="stretch"` instances still in `2_Graphs.py` (finding #5) which are direct copies of the pattern already corrected in `plotter.py` and `0_Home.py`. diff --git a/modules/aiprocessing.py b/modules/aiprocessing.py index 98afbf4..7afc7d4 100644 --- a/modules/aiprocessing.py +++ b/modules/aiprocessing.py @@ -3,7 +3,6 @@ import json import logging import re -import traceback import pandas as pd from anthropic import Anthropic @@ -17,7 +16,7 @@ logger = logging.getLogger(__name__) # Default model — can be overridden by settings.json key "AI.model" -_DEFAULT_CLAUDE_MODEL = "claude-sonnet-4-20250514" +_DEFAULT_CLAUDE_MODEL = "claude-sonnet-4-6-20251022" # Module-level singleton; rebuilt only when the API key changes. _anthropic_client: Anthropic | None = None @@ -165,7 +164,7 @@ def call_ai(messages: list, api_key: str, system_prompt: str = "", model: str | ) except Exception as e: logger.error("API call failed: %s", str(e)) - traceback.print_exc() + logger.exception("Anthropic API call failed") return None, None # Extract text content from response diff --git a/modules/cmc.py b/modules/cmc.py index 4678d5c..3935d5d 100644 --- a/modules/cmc.py +++ b/modules/cmc.py @@ -1,6 +1,5 @@ from datetime import datetime import logging -import traceback import pytz import requests @@ -8,7 +7,7 @@ class CMC: - def __init__(self, coinmarketcap_token: str) -> dict: + def __init__(self, coinmarketcap_token: str) -> None: self.coinmarketcap_token = coinmarketcap_token def get_current_fiat_prices( @@ -42,7 +41,11 @@ def get_current_fiat_prices( "convert": names, } - response = requests.get(url, headers=headers, params=params, timeout=10) + try: + response = requests.get(url, headers=headers, params=params, timeout=10) + except requests.exceptions.RequestException as e: + logger.error("Erreur réseau CMC (fiat) : %s", e) + return None if response.status_code == 200: fiat_prices = {} content = response.json() @@ -69,7 +72,7 @@ def get_current_fiat_prices( except (KeyError, IndexError, TypeError) as e: logger.error("Error getting price for %s : %s", fiat, str(e)) - traceback.print_exc() + logger.exception("Failed to parse fiat price for %s", fiat) logger.debug("Data received: %s", content["data"]) return None return fiat_prices @@ -107,7 +110,11 @@ def get_crypto_prices(self, tokens: list, unit="EUR", debug=False): "convert": unit, } - response = requests.get(url, headers=headers, params=params, timeout=10) + try: + response = requests.get(url, headers=headers, params=params, timeout=10) + except requests.exceptions.RequestException as e: + logger.error("Erreur réseau CMC (crypto) : %s", e) + return None if response.status_code == 200: logger.info("Get current market prices from Coinmarketcap successfully") content = response.json() diff --git a/modules/configuration.py b/modules/configuration.py index 717a053..b84993d 100644 --- a/modules/configuration.py +++ b/modules/configuration.py @@ -41,11 +41,41 @@ def save_config(self, settings: dict): settings: Dictionary containing settings to save """ logger.debug("Saving configuration") + + project_root = os.path.abspath(os.getcwd()) + + def _project_relative(path: str) -> str: + """Return a project-relative path when possible, preserving subfolders.""" + abs_path = os.path.abspath(path) + return os.path.relpath(abs_path, project_root) + + def _strip_debug_prefix(path: str) -> str: + """Remove debug_ prefix only from final filename component.""" + directory, filename = os.path.split(path) + if filename.startswith("debug_"): + filename = filename[len("debug_") :] + return os.path.join(directory, filename) if directory else filename + + data_path = _project_relative(settings["data_path"]) + archive_path = _strip_debug_prefix(_project_relative(settings["archive_path"])) + + db_abs = os.path.abspath(settings["dbfile"]) + data_abs = os.path.abspath(settings["data_path"]) + sqlite_file = os.path.relpath(db_abs, data_abs) + if sqlite_file.startswith(".."): + sqlite_file = os.path.basename(db_abs) + sqlite_file = _strip_debug_prefix(sqlite_file) + config = { "MarketRaccoon": { "url": settings["marketraccoon_url"], "token": settings.get("marketraccoon_token", ""), }, + "RatesDB": { + "url": settings.get( + "ratesdb_url", "https://free.ratesdb.com/v1/rates" + ) + }, "Notion": { "token": settings["notion_token"], "database": settings["notion_database"], @@ -55,13 +85,9 @@ def save_config(self, settings: dict): "AI": {"token": settings["ai_apitoken"]}, "Debug": {"flag": str(settings["debug_flag"])}, "Local": { - "archive_path": os.path.basename(settings["archive_path"]).replace( - "debug_", "" - ), - "data_path": os.path.basename(settings["data_path"]), - "sqlite_file": os.path.basename(settings["dbfile"]).replace( - "debug_", "" - ), + "archive_path": archive_path, + "data_path": data_path, + "sqlite_file": sqlite_file, }, "OperationsColors": { "green_threshold": settings.get("operations_green_threshold", 100), diff --git a/modules/database/apimarket.py b/modules/database/apimarket.py index c7c2927..613710c 100644 --- a/modules/database/apimarket.py +++ b/modules/database/apimarket.py @@ -6,7 +6,7 @@ """ import logging -from datetime import datetime +from datetime import datetime, timezone from typing import Optional import pandas as pd @@ -179,7 +179,7 @@ def get_currency(self, timestamp: int = None) -> pd.DataFrame: # Convert Unix timestamp to ISO 8601 format dt = datetime.fromtimestamp(timestamp, tz=self.local_timezone) - date_str = dt.astimezone(pd.Timestamp.now(tz="UTC").tz).isoformat() + date_str = dt.astimezone(timezone.utc).isoformat() headers = {} if self.api_key: @@ -369,12 +369,10 @@ def get_cryptocurrency_market( params = {"coinid": coinid} if from_timestamp: dt = datetime.fromtimestamp(from_timestamp, tz=self.local_timezone) - params["startdate"] = dt.astimezone( - pd.Timestamp.now(tz="UTC").tz - ).isoformat() + params["startdate"] = dt.astimezone(timezone.utc).isoformat() if to_timestamp: dt = datetime.fromtimestamp(to_timestamp, tz=self.local_timezone) - params["enddate"] = dt.astimezone(pd.Timestamp.now(tz="UTC").tz).isoformat() + params["enddate"] = dt.astimezone(timezone.utc).isoformat() # Fetch all pages headers = {} diff --git a/modules/database/fiat_cache.py b/modules/database/fiat_cache.py index 773f4fe..27eb46f 100644 --- a/modules/database/fiat_cache.py +++ b/modules/database/fiat_cache.py @@ -50,7 +50,7 @@ def __init__(self, cache_file: str, ttl_seconds: int = 3600): cache_dir = os.path.dirname(cache_file) if cache_dir and not os.path.exists(cache_dir): os.makedirs(cache_dir, exist_ok=True) - logger.info(f"Created cache directory: {cache_dir}") + logger.info("Created cache directory: %s", cache_dir) def get(self, key: str) -> Optional[dict]: """Get cached value if it exists and is not expired. @@ -64,17 +64,17 @@ def get(self, key: str) -> Optional[dict]: cache_data = self._load_cache() if key not in cache_data: - logger.debug(f"Cache MISS for key: {key} (not found)") + logger.debug("Cache MISS for key: %s (not found)", key) return None entry = cache_data[key] current_time = int(time.time()) if current_time >= entry["expiry"]: - logger.debug(f"Cache MISS for key: {key} (expired)") + logger.debug("Cache MISS for key: %s (expired)", key) return None - logger.debug(f"Cache HIT for key: {key}") + logger.debug("Cache HIT for key: %s", key) return entry["data"] def set(self, key: str, value: dict, ttl_override: Optional[int] = None): @@ -96,7 +96,7 @@ def set(self, key: str, value: dict, ttl_override: Optional[int] = None): "data": value, } - logger.debug(f"Cache SET for key: {key} (TTL: {ttl}s)") + logger.debug("Cache SET for key: %s (TTL: %ss)", key, ttl) # Cleanup expired entries before saving cache_data = self._cleanup_expired_entries(cache_data) @@ -149,33 +149,34 @@ def get_or_fetch( # Strategy 1: Try fresh cache cached = self.get(key) if cached is not None: - logger.debug(f"Using fresh cache for key: {key}") + logger.debug("Using fresh cache for key: %s", key) return cached # Strategy 2: Try to fetch fresh data try: - logger.debug(f"Cache miss for key: {key}, fetching from source") + logger.debug("Cache miss for key: %s, fetching from source", key) fresh_data = fetch_func() if fresh_data is not None: self.set(key, fresh_data) - logger.info(f"Fetched and cached fresh data for key: {key}") + logger.info("Fetched and cached fresh data for key: %s", key) return fresh_data else: - logger.warning(f"Fetch returned None for key: {key}") + logger.warning("Fetch returned None for key: %s", key) except Exception as e: - logger.error(f"Failed to fetch data for key {key}: {e}") + logger.error("Failed to fetch data for key %s: %s", key, e) # Strategy 3: Fallback to expired cache if allowed if allow_expired_fallback: expired_data = self._get_expired(key) if expired_data is not None: logger.warning( - f"Using EXPIRED cache for key: {key} due to fetch failure" + "Using EXPIRED cache for key: %s due to fetch failure", + key, ) return expired_data - logger.error(f"All strategies failed for key: {key}") + logger.error("All strategies failed for key: %s", key) return None def cleanup_expired(self): @@ -191,7 +192,7 @@ def cleanup_expired(self): if cleaned_count > 0: self._save_cache(cache_data) - logger.info(f"Cleaned {cleaned_count} expired entries from cache") + logger.info("Cleaned %d expired entries from cache", cleaned_count) else: logger.debug("No expired entries to clean") @@ -218,25 +219,25 @@ def _load_cache(self) -> dict: Cache data dict, or empty dict if file doesn't exist or is corrupted """ if not os.path.exists(self.cache_file): - logger.debug(f"Cache file does not exist: {self.cache_file}") + logger.debug("Cache file does not exist: %s", self.cache_file) return {} try: with open(self.cache_file, "r", encoding="utf-8") as f: cache_data = json.load(f) - logger.debug(f"Loaded cache with {len(cache_data)} entries") + logger.debug("Loaded cache with %d entries", len(cache_data)) return cache_data except (json.JSONDecodeError, IOError) as e: - logger.error(f"Cache file corrupted: {e}, resetting cache") + logger.error("Cache file corrupted: %s, resetting cache", e) # Backup corrupted file for debugging timestamp = int(time.time()) backup_file = f"{self.cache_file}.corrupted.{timestamp}" try: shutil.copy(self.cache_file, backup_file) - logger.info(f"Corrupted cache backed up to: {backup_file}") + logger.info("Corrupted cache backed up to: %s", backup_file) except Exception as backup_error: - logger.error(f"Failed to backup corrupted cache: {backup_error}") + logger.error("Failed to backup corrupted cache: %s", backup_error) return {} @@ -250,6 +251,7 @@ def _save_cache(self, cache_data: dict): cache_data: Cache data dict to save """ cache_dir = os.path.dirname(self.cache_file) + temp_path: str | None = None # Create temp file in same directory for atomic rename try: @@ -263,12 +265,12 @@ def _save_cache(self, cache_data: dict): # Atomic rename (OS-level atomic operation) shutil.move(temp_path, self.cache_file) - logger.debug(f"Saved cache with {len(cache_data)} entries") + logger.debug("Saved cache with %d entries", len(cache_data)) except Exception as e: - logger.error(f"Failed to save cache: {e}") + logger.error("Failed to save cache: %s", e) # Cleanup temp file if it exists - if "temp_path" in locals() and os.path.exists(temp_path): + if temp_path is not None and os.path.exists(temp_path): try: os.unlink(temp_path) except Exception: @@ -294,7 +296,7 @@ def _cleanup_expired_entries(self, cache_data: dict) -> dict: removed_count = len(cache_data) - len(cleaned) if removed_count > 0: - logger.debug(f"Cleaned {removed_count} expired entries") + logger.debug("Cleaned %d expired entries", removed_count) return cleaned @@ -317,6 +319,6 @@ def _evict_oldest(self, cache_data: dict) -> dict: evict_count = len(cache_data) - self.max_entries kept_items = sorted_items[evict_count:] - logger.warning(f"Evicted {evict_count} oldest entries (LRU)") + logger.warning("Evicted %d oldest entries (LRU)", evict_count) return dict(kept_items) diff --git a/modules/database/market.py b/modules/database/market.py index 4e86020..79f7942 100644 --- a/modules/database/market.py +++ b/modules/database/market.py @@ -19,6 +19,8 @@ logger = logging.getLogger(__name__) +DEFAULT_RATESDB_URL = "https://free.ratesdb.com/v1/rates" + _DROP_DUPLICATE_QUERIES = { "TokensDatabase": "SELECT * FROM TokensDatabase;", "Market": "SELECT * FROM Market;", @@ -30,15 +32,19 @@ class Market: """Class for managing cryptocurrency market data and currency rates.""" - def __init__(self, db_path: str, cmc_token: str): + def __init__( + self, db_path: str, cmc_token: str, ratesdb_url: str = DEFAULT_RATESDB_URL + ): """Initialize Market instance. Args: db_path: Path to SQLite database file cmc_token: CoinMarketCap API token + ratesdb_url: Base URL for historical fiat rates API """ self.db_path = db_path self.cmc_token = cmc_token + self.ratesdb_url = ratesdb_url.rstrip("/") self.__init_database() self.local_timezone = tzlocal.get_localzone() @@ -166,7 +172,8 @@ def update_market(self, tokens: list = None, debug: bool = False): logger.debug("Add tokens") known_tokens = self.get_tokens() - tokens = list(set(tokens + known_tokens)) + all_tokens = set(tokens + known_tokens) + tokens = list(all_tokens) logger.debug("tokens: %s", str(tokens)) timestamp = int(pd.Timestamp.now(tz=pytz.UTC).timestamp()) @@ -210,7 +217,7 @@ def get_price(self, token: str, timestamp: int = None) -> float: Token price or 0.0 if not found """ with sqlite3.connect(self.db_path) as con: - if timestamp: + if timestamp is not None: df = pd.read_sql_query( "SELECT price FROM Market WHERE token = ? AND timestamp <= ? ORDER BY timestamp DESC LIMIT 1", con, @@ -260,7 +267,8 @@ def drop_duplicate(self, table: str): if dupcount > 0: logger.debug("Found %d duplicated rows. Dropping...", dupcount) df.drop_duplicates(inplace=True) - df.to_sql(table, con, if_exists="replace", index=False) + con.execute(f"DELETE FROM {table}") # noqa: S608 + df.to_sql(table, con, if_exists="append", index=False) def __find_missing_timestamps(self) -> pd.DataFrame: """Find missing timestamps in the Currency table. @@ -312,61 +320,114 @@ def update_currencies(self, debug: bool = False): if count == 0: logger.debug("No missing timestamps") else: - idx = 0 - for timestamp in df_timestamps["timestamp"]: - idx += 1 - # convert timestamp to datetime(YYYY-MM-DD) - date = pd.to_datetime(timestamp, unit="s", utc=True).strftime( - "%Y-%m-%d" + # Build unique date -> canonical rate timestamp mapping once. + # This avoids one HTTP request per raw timestamp when several map to the same day. + ts_series = pd.to_datetime(df_timestamps["timestamp"], unit="s", utc=True) + date_to_rate_ts = { + d.strftime("%Y-%m-%d"): int( + pd.Timestamp(f"{d.strftime('%Y-%m-%d')} 14:30:00+00:00").timestamp() ) - rate_date = f"{date} 14:30:00+00:00" - rate_timestamp = int(pd.Timestamp(rate_date).timestamp()) + for d in ts_series + } + + rows_to_insert: list[tuple[int, str, float]] = [] + session = requests.Session() + dates = sorted(date_to_rate_ts.keys()) + logger.debug("Missing timestamps: %d, unique dates to fetch: %d", count, len(dates)) + + for idx, date in enumerate(dates, start=1): + rate_timestamp = date_to_rate_ts[date] logger.debug( - "%d/%d Timestamp: %d -> Date: %s -> Rate Date: %s -> Rate Timestamp: %d", + "%d/%d Date: %s -> Rate Timestamp: %d", idx, - count, - timestamp, + len(dates), date, - rate_date, rate_timestamp, ) - # request the currency rate - url = f"https://free.ratesdb.com/v1/rates?from=EUR&to=USD&date={date}" - response = requests.get(url, timeout=10) - if response.status_code != 200: + url = f"{self.ratesdb_url}?from=EUR&to=USD&date={date}" + + # Retry only on transient errors (429/5xx or network errors). + max_attempts = 3 + resp = None + for attempt in range(1, max_attempts + 1): + try: + response = session.get(url, timeout=10) + except requests.exceptions.RequestException as e: + logger.warning( + "Currency request failed for %s (attempt %d/%d): %s", + date, + attempt, + max_attempts, + e, + ) + if attempt < max_attempts: + time.sleep(attempt) + continue + + if response.status_code == 200: + try: + resp = response.json() + except ValueError: + logger.error( + "Invalid JSON response from currency API for date %s", + date, + ) + break + + if response.status_code == 429 or response.status_code >= 500: + logger.warning( + "Transient currency API status for %s: %d (attempt %d/%d)", + date, + response.status_code, + attempt, + max_attempts, + ) + if attempt < max_attempts: + time.sleep(attempt) + continue + logger.error( - "Error updating currencies. Code: %d", response.status_code + "Error updating currencies for %s. Code: %d", + date, + response.status_code, ) - time.sleep(1) + break + + if resp is None: continue - try: - resp = response.json() - except ValueError: - logger.error( - "Invalid JSON response from currency API for date %s", date - ) - time.sleep(1) + + usd_rate = resp.get("data", {}).get("rates", {}).get("USD") + if usd_rate is None: + logger.error("Missing USD rate in response for date %s", date) continue - logger.debug( - "Rate Timestamp: %d - Rate: %f", - rate_timestamp, - resp["data"]["rates"]["USD"], - ) - self.add_currency(rate_timestamp, "USD", resp["data"]["rates"]["USD"]) + logger.debug("Rate Timestamp: %d - Rate: %f", rate_timestamp, usd_rate) + rows_to_insert.append((rate_timestamp, "USD", float(usd_rate))) - # sleep 1 second to avoid api request rate limit - time.sleep(1) + if rows_to_insert: + with sqlite3.connect(self.db_path) as con: + cur = con.cursor() + cur.executemany( + "INSERT INTO Currency (timestamp, currency, price) VALUES (?, ?, ?)", + rows_to_insert, + ) + con.commit() + logger.info("Inserted %d historical currency rates", len(rows_to_insert)) + else: + logger.warning("No historical currency rates inserted") # add current rate to Currency from CMC cmc_prices = CMC(self.cmc_token) price = cmc_prices.get_current_fiat_prices(debug=debug) logger.debug("Adding current rate to Currency: %s", price) - for currency in price: - self.add_currency( - price[currency]["timestamp"], currency, price[currency]["price"] - ) + if price is None: + logger.warning("CMC fiat prices unavailable, skipping currency update") + else: + for currency in price: + self.add_currency( + price[currency]["timestamp"], currency, price[currency]["price"] + ) # drop duplicate self.drop_duplicate("Currency") diff --git a/modules/database/migrations.py b/modules/database/migrations.py index 5b6ef57..38b4d15 100644 --- a/modules/database/migrations.py +++ b/modules/database/migrations.py @@ -50,45 +50,34 @@ def _get_db_version(db_path: str) -> int: return 0 -def _set_db_version(db_path: str, version: int) -> None: - """Enregistre la version courante dans Customdata.""" - try: - with sqlite3.connect(db_path) as conn: - conn.execute( - "INSERT OR REPLACE INTO Customdata (name, value, type) VALUES ('db_version', ?, 'int')", - (str(version),), - ) - except sqlite3.Error as e: - logger.error("Erreur lors de l'écriture de db_version : %s", e) - raise - - def _migrate_v1(conn: sqlite3.Connection) -> None: """Schéma original — crée toutes les tables de base.""" - conn.executescript( - """ - CREATE TABLE IF NOT EXISTS TokensDatabase ( + conn.execute( + """CREATE TABLE IF NOT EXISTS TokensDatabase ( timestamp INTEGER, token TEXT, price REAL, count REAL - ); - - CREATE TABLE IF NOT EXISTS Portfolios ( + )""" + ) + conn.execute( + """CREATE TABLE IF NOT EXISTS Portfolios ( id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT NOT NULL UNIQUE, bundle INTEGER DEFAULT 0 - ); - - CREATE TABLE IF NOT EXISTS Portfolios_Tokens ( + )""" + ) + conn.execute( + """CREATE TABLE IF NOT EXISTS Portfolios_Tokens ( portfolio_id INTEGER NOT NULL, token TEXT NOT NULL, amount REAL, PRIMARY KEY (portfolio_id, token), FOREIGN KEY (portfolio_id) REFERENCES Portfolios(id) - ); - - CREATE TABLE IF NOT EXISTS Operations ( + )""" + ) + conn.execute( + """CREATE TABLE IF NOT EXISTS Operations ( id INTEGER PRIMARY KEY AUTOINCREMENT, type TEXT, source REAL, @@ -97,23 +86,27 @@ def _migrate_v1(conn: sqlite3.Connection) -> None: destination_unit TEXT, timestamp INTEGER, portfolio TEXT - ); - - CREATE TABLE IF NOT EXISTS Market ( + )""" + ) + conn.execute( + """CREATE TABLE IF NOT EXISTS Market ( timestamp INTEGER, token TEXT, price REAL - ); - - CREATE TABLE IF NOT EXISTS Currency ( + )""" + ) + conn.execute( + """CREATE TABLE IF NOT EXISTS Currency ( timestamp INTEGER, currency TEXT, price REAL - ); - - CREATE INDEX IF NOT EXISTS idx_currency ON Currency (timestamp, currency); - - CREATE TABLE IF NOT EXISTS Swaps ( + )""" + ) + conn.execute( + "CREATE INDEX IF NOT EXISTS idx_currency ON Currency (timestamp, currency)" + ) + conn.execute( + """CREATE TABLE IF NOT EXISTS Swaps ( id INTEGER PRIMARY KEY AUTOINCREMENT, timestamp INTEGER, token_from TEXT, @@ -123,9 +116,10 @@ def _migrate_v1(conn: sqlite3.Connection) -> None: amount_to REAL, wallet_to TEXT, tag TEXT - ); - - CREATE TABLE IF NOT EXISTS TokenMetadata ( + )""" + ) + conn.execute( + """CREATE TABLE IF NOT EXISTS TokenMetadata ( token TEXT PRIMARY KEY, status TEXT, delisting_date INTEGER, @@ -133,8 +127,7 @@ def _migrate_v1(conn: sqlite3.Connection) -> None: notes TEXT, created_at INTEGER DEFAULT (strftime('%s', 'now')), updated_at INTEGER DEFAULT (strftime('%s', 'now')) - ); - """ + )""" ) @@ -220,6 +213,37 @@ def _migrate_v6(conn: sqlite3.Connection) -> None: ) +def _migrate_v7(conn: sqlite3.Connection) -> None: + """Conversion des colonnes amount_from/amount_to de TEXT en REAL dans Swaps.""" + conn.execute( + """CREATE TABLE IF NOT EXISTS Swaps_new ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + timestamp INTEGER, + token_from TEXT, + amount_from REAL, + wallet_from TEXT, + token_to TEXT, + amount_to REAL, + wallet_to TEXT, + tag TEXT, + note TEXT + )""" + ) + conn.execute( + """INSERT INTO Swaps_new + (id, timestamp, token_from, amount_from, wallet_from, + token_to, amount_to, wallet_to, tag, note) + SELECT id, timestamp, token_from, + CAST(amount_from AS REAL), + wallet_from, token_to, + CAST(amount_to AS REAL), + wallet_to, tag, note + FROM Swaps""" + ) + conn.execute("DROP TABLE Swaps") + conn.execute("ALTER TABLE Swaps_new RENAME TO Swaps") + + MIGRATIONS: dict = { 1: _migrate_v1, 2: _migrate_v2, @@ -227,6 +251,7 @@ def _migrate_v6(conn: sqlite3.Connection) -> None: 4: _migrate_v4, 5: _migrate_v5, 6: _migrate_v6, + 7: _migrate_v7, } diff --git a/modules/database/portfolios.py b/modules/database/portfolios.py index 529beab..380432d 100644 --- a/modules/database/portfolios.py +++ b/modules/database/portfolios.py @@ -25,12 +25,12 @@ def get_portfolio_names(self) -> list: cursor.execute("SELECT name FROM Portfolios") # return sorted list of portfolios portfolio_names = [row[0] for row in cursor.fetchall()] - logger.debug(f"Getting portfolios from database {portfolio_names}") + logger.debug("Getting portfolios from database %s", portfolio_names) portfolio_names.sort() return portfolio_names def get_portfolio(self, name: str) -> dict: - logger.debug(f"Getting portfolio {name} from database") + logger.debug("Getting portfolio %s from database", name) with sqlite3.connect(self.db_path) as conn: cursor = conn.cursor() cursor.execute( @@ -103,7 +103,7 @@ def set_token(self, portfolio_name: str, token: str, amount: float): ? ) """, - (portfolio_name, token, str(amount)), + (portfolio_name, token, amount), ) def set_token_add(self, name: str, token: str, amount: float): @@ -127,7 +127,7 @@ def set_token_add(self, name: str, token: str, amount: float): SET amount = ? WHERE portfolio_id = (SELECT id FROM Portfolios WHERE name = ?) AND token = ? """, - (str(new_amount), name, token), + (new_amount, name, token), ) else: cursor.execute( diff --git a/modules/database/swaps.py b/modules/database/swaps.py index ce925f7..a654748 100644 --- a/modules/database/swaps.py +++ b/modules/database/swaps.py @@ -25,10 +25,10 @@ def __init__(self, db_path: str): id INTEGER PRIMARY KEY AUTOINCREMENT, timestamp INTEGER, token_from TEXT, - amount_from TEXT, + amount_from REAL, wallet_from TEXT, token_to TEXT, - amount_to TEXT, + amount_to REAL, wallet_to TEXT, tag TEXT, note TEXT @@ -112,7 +112,7 @@ def delete(self, entry_id: int): cursor = conn.cursor() cursor.execute("DELETE FROM Swaps WHERE id = ?", (entry_id,)) conn.commit() - logger.debug(f"Entry with id {entry_id} deleted successfully.") + logger.debug("Entry with id %s deleted successfully.", entry_id) except Exception as e: logger.exception("Error deleting swap: %s", e) @@ -124,7 +124,7 @@ def update_note(self, entry_id: int, note: str): "UPDATE Swaps SET note = ? WHERE id = ?", (note or None, entry_id) ) conn.commit() - logger.debug(f"Note updated for entry with id {entry_id}") + logger.debug("Note updated for entry with id %s", entry_id) except Exception as e: logger.exception("Error updating note: %s", e) @@ -142,6 +142,6 @@ def update_tag(self, entry_id: int, tag: str): "UPDATE Swaps SET tag = ? WHERE id = ?", (tag, entry_id) ) conn.commit() - logger.debug(f"Tag updated for entry with id {entry_id}") + logger.debug("Tag updated for entry with id %s", entry_id) except Exception as e: logger.exception("Error updating tag: %s", e) diff --git a/modules/database/tokensdb.py b/modules/database/tokensdb.py index f1d1582..0ff46e5 100644 --- a/modules/database/tokensdb.py +++ b/modules/database/tokensdb.py @@ -45,7 +45,6 @@ def get_sum_over_time(self) -> pd.DataFrame: df_sum.rename(columns={"timestamp": "Date", "value": "Sum"}, inplace=True) df_sum.set_index("Date", inplace=True) df_sum.sort_index(inplace=True) - df_sum = df_sum.reindex(sorted(df_sum.columns), axis=1) logger.debug("Final Sums:\n%s", df_sum) return df_sum @@ -92,9 +91,6 @@ def get_balances(self) -> pd.DataFrame: df_balance.rename(columns={"timestamp": "Date"}, inplace=True) df_balance.set_index("Date", inplace=True) df_balance.sort_index(inplace=True) - - # Sort columns alphabetically - df_balance = df_balance.reindex(sorted(df_balance.columns), axis=1) logger.debug("Balances shape: %s", df_balance.shape) logger.debug("Balances:\n%s", df_balance.head()) return df_balance @@ -225,7 +221,8 @@ def drop_duplicate(self): if dupcount > 0: logger.debug("Found %d duplicated rows. Dropping...", dupcount) df.drop_duplicates(inplace=True) - df.to_sql("TokensDatabase", con, if_exists="replace", index=False) + con.execute("DELETE FROM TokensDatabase") + df.to_sql("TokensDatabase", con, if_exists="append", index=False) def get_tokens(self) -> list: with sqlite3.connect(self.db_path) as con: diff --git a/modules/plotter.py b/modules/plotter.py index 3821311..12203ef 100644 --- a/modules/plotter.py +++ b/modules/plotter.py @@ -15,22 +15,22 @@ def plot_as_pie(df: pd.DataFrame, column): total = df[column].sum() limit = total / 100 - logger.debug(f"1% of {total} is {limit}") + logger.debug("1%% of %s is %s", total, limit) # Group token representing less then 1% of total value dffinal = df.loc[df[column] >= limit] - logger.debug(f"Dataframe more than {limit}:\n{dffinal}") + logger.debug("Dataframe more than %s:\n%s", limit, dffinal) dfless = df.loc[df[column] < limit] if not dfless.empty: - logger.debug(f"Dataframe less than {limit}:\n{dfless}") + logger.debug("Dataframe less than %s:\n%s", limit, dfless) dfless_sum = pd.DataFrame(dfless.sum()).T dfless_sum.index = ["Others"] - logger.debug(f"Dataframe less than 1% sum:\n{dfless_sum}") + logger.debug("Dataframe less than 1%% sum:\n%s", dfless_sum) dffinal = pd.concat([dffinal, dfless_sum]) - logger.debug(f"Dataframe more than 1% sum with Others:\n{dffinal}") + logger.debug("Dataframe more than 1%% sum with Others:\n%s", dffinal) fig = px.pie(dffinal, dffinal.index, column, width=700, height=700) st.plotly_chart(fig, use_container_width=True) @@ -64,4 +64,4 @@ def plot_as_graph(df: pd.DataFrame): # Définir le titre de l'axe y par défaut pour la première colonne fig.update_layout(yaxis={"title": df.columns[0]}) - st.plotly_chart(fig) + st.plotly_chart(fig, use_container_width=True) diff --git a/modules/token_metadata.py b/modules/token_metadata.py index 6ab290d..48b83c9 100644 --- a/modules/token_metadata.py +++ b/modules/token_metadata.py @@ -22,7 +22,7 @@ class TokenStatus(Enum): class TokenMetadataManager: """Gestionnaire des métadonnées des tokens""" - def __init__(self, db_path: str = "data/db.sqlite3"): + def __init__(self, db_path: str): self.db_path = db_path def get_token_status(self, token: str) -> Optional[TokenStatus]: diff --git a/modules/tools.py b/modules/tools.py index 48e6a90..c6df717 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -13,7 +13,6 @@ import os import shutil import sqlite3 -import traceback import numpy as np import pandas as pd @@ -198,8 +197,7 @@ def convert_dataframe_prices_historical( else: # EUR → USD converted = merged["price_value"] / merged["rate"].replace(0, pd.NA) - # Remettre les valeurs converties dans le DataFrame original (même ordre d'index) - df[price_column] = converted.values + df[price_column] = converted.set_axis(merged["Date"]) return df @@ -233,9 +231,16 @@ def get_cached_api_market() -> ApiMarket: def update_database(dbfile: str, cmc_apikey: str, debug: bool): """Update the database with the latest market data""" - backup_database(dbfile) + if os.path.exists(dbfile): + backup_database(dbfile) - market = Market(dbfile, cmc_apikey) + market = Market( + dbfile, + cmc_apikey, + ratesdb_url=st.session_state.settings.get( + "ratesdb_url", "https://free.ratesdb.com/v1/rates" + ), + ) portfolio = Portfolios(dbfile) aggregated = portfolio.aggregate_portfolios() @@ -256,7 +261,7 @@ def update_database(dbfile: str, cmc_apikey: str, debug: bool): market.update_currencies(debug=debug) except Exception as e: logger.error("Error updating market data: %s", str(e)) - traceback.print_exc() + logger.exception("Error updating market data") raise ValueError("Error updating market data") from e tokens_prices = market.get_last_market() @@ -278,7 +283,7 @@ def update_database(dbfile: str, cmc_apikey: str, debug: bool): custom = Customdata(dbfile) custom.set( - "last_update", str(int(pd.Timestamp.now(tz="UTC").timestamp())), "integer" + "last_update", str(int(pd.Timestamp.now(tz="UTC").timestamp())), "int" ) @@ -293,7 +298,7 @@ def parse_last_update(last_update_data: tuple) -> pd.Timestamp: """ value, value_type = last_update_data - if value_type == "integer": + if value_type in ("int", "integer"): timestamp = int(value) elif value_type == "float": timestamp = int(float(value)) @@ -375,6 +380,9 @@ def load_settings(settings: dict): st.session_state.settings["marketraccoon_token"] = settings["MarketRaccoon"].get( "token", "" ) + st.session_state.settings["ratesdb_url"] = settings.get("RatesDB", {}).get( + "url", "https://free.ratesdb.com/v1/rates" + ) st.session_state.settings["notion_token"] = settings["Notion"]["token"] st.session_state.settings["notion_database"] = settings["Notion"]["database"] st.session_state.settings["notion_parentpage"] = settings["Notion"]["parentpage"] @@ -694,7 +702,7 @@ def _get_api_fiat_rate() -> float: USD to EUR rate (e.g. 0.85 means 1 USD = 0.85 EUR), or None on error """ api = get_cached_api_market() - fiat_df = api.get_fiat_latest_rate() + fiat_df = api.get_fiat_latest_rate_cached() if fiat_df is not None and not fiat_df.empty: rate = fiat_df.iloc[-1]["price"] logger.info("API fiat rate USD→EUR: %s", rate) @@ -985,7 +993,7 @@ def update(): st.rerun() except (ConnectionError, ValueError) as e: st.error(f"Update Error: {str(e)}") - traceback.print_exc() + logger.exception("Price update failed") def backup_database(dbfile: str, max_backups: int = 10) -> str: diff --git a/modules/utils.py b/modules/utils.py index 759c69f..6611dea 100644 --- a/modules/utils.py +++ b/modules/utils.py @@ -10,7 +10,7 @@ logger = logging.getLogger(__name__) -def __find_linear_function(x1, y1, x2, y2): +def _find_linear_function(x1, y1, x2, y2): # Calculer la pente a a = (y2 - y1) / (x2 - x1) # Calculer l'ordonnée à l'origine b @@ -26,7 +26,7 @@ def interpolate(x1, y1, x2, y2, x): y1, y2 = y2, y1 if x < x1 or x > x2: raise ValueError(f"x={x} is out of range [{x1}, {x2}]") - a, b = __find_linear_function(x1, y1, x2, y2) + a, b = _find_linear_function(x1, y1, x2, y2) return a * x + b From e014384271c61a0e4ddd8ba75a550b345eb3b4a3 Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Sat, 18 Apr 2026 13:13:37 +0300 Subject: [PATCH 12/13] fix(lint): remove unused exception var in X_Tests.py --- app_pages/X_Tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app_pages/X_Tests.py b/app_pages/X_Tests.py index 7d0f253..de4c6c5 100644 --- a/app_pages/X_Tests.py +++ b/app_pages/X_Tests.py @@ -173,7 +173,7 @@ def check_api_data_for_date(api_url: str, api_key: str, date) -> bool: try: has_data = future.result() api_status_dict[date] = "✅" if has_data else "❌" - except Exception as e: + except Exception: logger.exception("Error checking date %s", date) api_status_dict[date] = "❌" From 0c8e96c007bc41e971693ff0267dd00ed301ba0f Mon Sep 17 00:00:00 2001 From: "Hansi P." Date: Sat, 18 Apr 2026 13:16:06 +0300 Subject: [PATCH 13/13] fix(lint): resolve all pylint warnings to reach 10.00/10 --- app_pages/3_Operations.py | 18 +++++++++--------- app_pages/6_Settings.py | 2 +- modules/aiprocessing.py | 6 +++--- modules/database/market.py | 18 ++++++++++-------- modules/database/swaps.py | 24 ++++++++++-------------- 5 files changed, 33 insertions(+), 35 deletions(-) diff --git a/app_pages/3_Operations.py b/app_pages/3_Operations.py index ea07e40..24bb6b4 100644 --- a/app_pages/3_Operations.py +++ b/app_pages/3_Operations.py @@ -973,30 +973,30 @@ def color_rows(row): styled_df = df.style.apply(color_rows, axis=1) # Build dynamic column order and config - COL_FROM_amount = ( + col_from_amount = ( f"From Amount ({convert_from})" if convert_from else "From Amount" ) - COL_TO_amount = f"To Amount ({convert_to})" if convert_to else "To Amount" - RATE_COL = "Swap Rate" + col_to_amount = f"To Amount ({convert_to})" if convert_to else "To Amount" + rate_col = "Swap Rate" column_order = [ "Date", - COL_FROM_amount, + col_from_amount, *(() if convert_from else ("From Token",)), - COL_TO_amount, + col_to_amount, *(() if convert_to else ("To Token",)), "From Wallet", "To Wallet", - RATE_COL, + rate_col, "Current Rate", "Perf.", "note", ] column_config = { - COL_FROM_amount: st.column_config.NumberColumn(format="%.8g"), - COL_TO_amount: st.column_config.NumberColumn(format="%.8g"), - RATE_COL: st.column_config.NumberColumn(format="%.8g"), + col_from_amount: st.column_config.NumberColumn(format="%.8g"), + col_to_amount: st.column_config.NumberColumn(format="%.8g"), + rate_col: st.column_config.NumberColumn(format="%.8g"), "Current Rate": st.column_config.NumberColumn(format="%.8g"), "Perf.": st.column_config.NumberColumn(format="%.2f%%"), } diff --git a/app_pages/6_Settings.py b/app_pages/6_Settings.py index fc79b65..f65f9f2 100644 --- a/app_pages/6_Settings.py +++ b/app_pages/6_Settings.py @@ -31,7 +31,7 @@ def _check_marketraccoon(url: str, api_key: str) -> str: with st.sidebar: st.subheader("Status") - _status = _check_marketraccoon( + _status = _check_marketraccoon( # pylint: disable=invalid-name st.session_state.settings.get("marketraccoon_url", ""), st.session_state.settings.get("marketraccoon_token", ""), ) diff --git a/modules/aiprocessing.py b/modules/aiprocessing.py index 7afc7d4..9ed917c 100644 --- a/modules/aiprocessing.py +++ b/modules/aiprocessing.py @@ -19,13 +19,13 @@ _DEFAULT_CLAUDE_MODEL = "claude-sonnet-4-6-20251022" # Module-level singleton; rebuilt only when the API key changes. -_anthropic_client: Anthropic | None = None -_anthropic_client_key: str | None = None +_anthropic_client: Anthropic | None = None # pylint: disable=invalid-name +_anthropic_client_key: str | None = None # pylint: disable=invalid-name def _get_client(api_key: str) -> Anthropic: """Return a cached Anthropic client, rebuilding only when the key changes.""" - global _anthropic_client, _anthropic_client_key + global _anthropic_client, _anthropic_client_key # pylint: disable=global-statement if _anthropic_client is None or _anthropic_client_key != api_key: _anthropic_client = Anthropic(api_key=api_key) _anthropic_client_key = api_key diff --git a/modules/database/market.py b/modules/database/market.py index 79f7942..1e049d1 100644 --- a/modules/database/market.py +++ b/modules/database/market.py @@ -424,10 +424,8 @@ def update_currencies(self, debug: bool = False): if price is None: logger.warning("CMC fiat prices unavailable, skipping currency update") else: - for currency in price: - self.add_currency( - price[currency]["timestamp"], currency, price[currency]["price"] - ) + for currency, data in price.items(): + self.add_currency(data["timestamp"], currency, data["price"]) # drop duplicate self.drop_duplicate("Currency") @@ -461,12 +459,14 @@ def get_token_lowhigh(self, token: str, timestamp: int) -> pd.DataFrame: """ with sqlite3.connect(self.db_path) as con: df_low = pd.read_sql_query( - "SELECT timestamp, price FROM Market WHERE token = ? AND timestamp <= ? ORDER BY timestamp DESC LIMIT 1", + "SELECT timestamp, price FROM Market" + " WHERE token = ? AND timestamp <= ? ORDER BY timestamp DESC LIMIT 1", con, params=(token, timestamp), ) df_high = pd.read_sql_query( - "SELECT timestamp, price FROM Market WHERE token = ? AND timestamp >= ? ORDER BY timestamp ASC LIMIT 1", + "SELECT timestamp, price FROM Market" + " WHERE token = ? AND timestamp >= ? ORDER BY timestamp ASC LIMIT 1", con, params=(token, timestamp), ) @@ -484,12 +484,14 @@ def get_currency_lowhigh(self, currency: str, timestamp: int) -> pd.DataFrame: """ with sqlite3.connect(self.db_path) as con: df_low = pd.read_sql_query( - "SELECT timestamp, price FROM Currency WHERE currency = ? AND timestamp <= ? ORDER BY timestamp DESC LIMIT 1", + "SELECT timestamp, price FROM Currency" + " WHERE currency = ? AND timestamp <= ? ORDER BY timestamp DESC LIMIT 1", con, params=(currency, timestamp), ) df_high = pd.read_sql_query( - "SELECT timestamp, price FROM Currency WHERE currency = ? AND timestamp >= ? ORDER BY timestamp ASC LIMIT 1", + "SELECT timestamp, price FROM Currency" + " WHERE currency = ? AND timestamp >= ? ORDER BY timestamp ASC LIMIT 1", con, params=(currency, timestamp), ) diff --git a/modules/database/swaps.py b/modules/database/swaps.py index a654748..d054c2b 100644 --- a/modules/database/swaps.py +++ b/modules/database/swaps.py @@ -54,19 +54,15 @@ def get_by_tag(self, tag: str) -> list: with sqlite3.connect(self.db_path) as conn: cursor = conn.cursor() + _sql = ( + "SELECT id, timestamp, token_from, amount_from, wallet_from," + " token_to, amount_to, wallet_to, tag, note FROM Swaps" + ) if not tag: - cursor.execute( - """ - SELECT id, timestamp, token_from, amount_from, wallet_from, token_to, amount_to, wallet_to, tag, note - FROM Swaps WHERE tag IS NULL ORDER BY timestamp DESC - """ - ) + cursor.execute(f"{_sql} WHERE tag IS NULL ORDER BY timestamp DESC") else: cursor.execute( - """ - SELECT id, timestamp, token_from, amount_from, wallet_from, token_to, amount_to, wallet_to, tag, note - FROM Swaps WHERE tag = ? ORDER BY timestamp DESC - """, + f"{_sql} WHERE tag = ? ORDER BY timestamp DESC", (tag,), ) return cursor.fetchall() @@ -86,10 +82,10 @@ def insert( with sqlite3.connect(self.db_path) as conn: cursor = conn.cursor() cursor.execute( - """ - INSERT INTO Swaps (timestamp, token_from, amount_from, wallet_from, token_to, amount_to, wallet_to, tag, note) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) - """, + "INSERT INTO Swaps" + " (timestamp, token_from, amount_from, wallet_from," + " token_to, amount_to, wallet_to, tag, note)" + " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)", ( timestamp, token_from,