fix: apply code review fixes across all modules#4
Merged
Conversation
- 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
- 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
- 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
- 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
- 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
- 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
…on logic for db_version handling
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
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.
- 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
traceback.print_exc()parlogger.exception()et suppression des importstraceback%sdans tous les modulesuse_container_width, date filtering,hash_funcs)amount_from/amount_tode la table Swaps convertis de TEXT en REALratesdb_urlconfigurable via settings/config/Market (fallback fiat rates)_get_api_fiat_rate(utilise maintenantget_fiat_latest_rate_cached)update_currencies: déduplication des dates, retry sur erreurs transitoires, insertion batchconfiguration.py(os.path.basename→os.path.relpath)code-review-report.mdTest plan