api: migrate exchange rate & market to Decimal #92

zensgit · 2025-10-15T14:22:22Z

Purpose

Migrate exchange rate and global market paths from f64 to Decimal to eliminate precision drift and align with Money/Decimal usage.

Scope

jive-api/src/services/exchange_rate_service.rs: ExchangeRate.rate → Decimal; external responses parsed into Decimal; DB storage uses Decimal directly
jive-api/src/services/exchange_rate_api.rs: External responses and defaults switched to Decimal; logging uses Decimal; removed ad‑hoc f64 conversions
jive-api/src/models/global_market.rs: API model HashMaps moved to Decimal; conversion code updated

Validation

SQLX_OFFLINE=true cargo clippy -- -D warnings passes locally
Unit tests pass (existing suite)

Notes

Percentages remain Decimal or are formatted at the edge; no DB migration required
Complements PR api: migrate transaction/budget money types to Decimal #90 (transactions/budget Decimal migration)

… and currency_service

…or dynamic SQLx reads for accounts + currency/exchange

…x IF NOT EXISTS for indexes/triggers

gemini-code-assist · 2025-10-15T14:22:48Z

Summary of Changes

Hello @zensgit, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves the numerical precision of financial data within the API by transitioning key fields from floating-point numbers (f64) to the Decimal type. This change is crucial for maintaining accuracy in calculations involving exchange rates, account balances, and other monetary values. The implementation involved refactoring database query patterns across multiple services and handlers to explicitly manage Decimal types, alongside making database migration scripts more resilient to repeated execution.

Highlights

Precision Migration to Decimal: Migrated various financial data points, including account balances, credit limits, and crypto exchange rates, from f64 to the Decimal type to eliminate precision drift and ensure consistency across the API.
SQLx Query Refactoring: Updated several API handlers and services to use sqlx::query instead of the sqlx::query! macro. This change allows for more explicit and robust handling of data types, particularly Decimal, when fetching and inserting data from the PostgreSQL database.
Database Migration Idempotency: Enhanced the add_net_worth_tracking.sql migration script to ensure idempotency for CREATE EXTENSION, CREATE INDEX, and CREATE TRIGGER statements, preventing errors on re-execution.
Improved Data Extraction and Defaults: Refined data extraction logic from SQL query results using row.get() and row.try_get(), providing clearer type conversions and robust default values for optional fields like currency, status, and timestamps.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request successfully migrates exchange rate and market data from f64 to Decimal to improve precision, which is a great improvement. The changes also refactor sqlx::query! macros to sqlx::query() calls to support offline compilation, though this has introduced a potential issue with error handling. The most critical issue is an unresolved merge conflict in jive-api/src/handlers/accounts.rs that must be fixed. Additionally, a widespread pattern of using .try_get(...).unwrap_or(None) silently swallows database errors, which could hide bugs. I've provided a suggestion to address this.

gemini-code-assist · 2025-10-15T14:24:53Z

jive-api/src/handlers/accounts.rs

+<<<<<<< HEAD
                  currency, current_balance, available_balance, credit_limit, status,
                  is_manual, color, notes, created_at, updated_at
+=======
+                  currency, current_balance, available_balance, credit_limit,
+                  status, is_manual, color, notes, created_at, updated_at
+>>>>>>> 46ef8086 (api: unify Decimal mapping in accounts handler; fix clippy in metrics and currency_service)


This file contains unresolved merge conflict markers (<<<<<<<, =======, >>>>>>>). These must be resolved before the pull request can be merged.

gemini-code-assist · 2025-10-15T14:24:53Z

jive-api/src/handlers/accounts.rs

+        currency: row
+            .try_get::<Option<String>, _>("currency")
+            .unwrap_or(None)
+            .unwrap_or_else(|| "CNY".to_string()),
+        current_balance: row
+            .try_get::<Option<Decimal>, _>("current_balance")
+            .unwrap_or(None)
+            .unwrap_or(Decimal::ZERO),


The pattern .try_get(...).unwrap_or(None) is used here and in several other places in this PR (e.g., create_account, get_account_statistics, and in currency_handler_enhanced.rs). This pattern can hide bugs by swallowing potential database errors. If try_get fails (for instance, due to a column name typo or a type mismatch), the Err is silently converted to None, and a default value is used. This prevents the API from reporting a server-side problem. It's safer to handle the Result from try_get explicitly, for example by using the ? operator to propagate the error, which will then be converted into a proper API error response.

Suggested change

currency: row

.try_get::<Option<String>, _>("currency")

.unwrap_or(None)

.unwrap_or_else(|| "CNY".to_string()),

current_balance: row

.try_get::<Option<Decimal>, _>("current_balance")

.unwrap_or(None)

.unwrap_or(Decimal::ZERO),

currency: row

.try_get::<Option<String>, _>("currency")?

.unwrap_or_else(|| "CNY".to_string()),

current_balance: row

.try_get::<Option<Decimal>, _>("current_balance")?

.unwrap_or(Decimal::ZERO),

Copilot

Pull Request Overview

Migrate exchange rate and related market paths from f64 to Decimal and switch several SQLx typed macros to dynamic queries for Decimal compatibility.

Replace sqlx::query! with sqlx::query and Row accessors to support Decimal columns.
Normalize timestamps with fallback defaults and minor logging/env handling tweaks.
Add migration hardening (IF NOT EXISTS for indexes/triggers and pgcrypto extension).

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
jive-api/src/services/exchange_rate_api.rs	Adjusted updated_at handling for age calculation.
jive-api/src/services/currency_service.rs	Switched to dynamic Row accessors and standardized time fallbacks.
jive-api/src/metrics.rs	Minor env var handling cleanup.
jive-api/src/handlers/currency_handler_enhanced.rs	Switched to dynamic query and Decimal handling for crypto prices.
jive-api/src/handlers/accounts.rs	Converted to dynamic queries, Decimal mapping; added casts in update; response mapping via Row getters.
jive-api/migrations/037_add_net_worth_tracking.sql	Hardened migration with IF NOT EXISTS and conditional trigger creation; ensured pgcrypto extension.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-15T14:25:27Z

jive-api/src/handlers/accounts.rs

+<<<<<<< HEAD
                  currency, current_balance, available_balance, credit_limit, status,
                  is_manual, color, notes, created_at, updated_at
+=======
+                  currency, current_balance, available_balance, credit_limit,
+                  status, is_manual, color, notes, created_at, updated_at
+>>>>>>> 46ef8086 (api: unify Decimal mapping in accounts handler; fix clippy in metrics and currency_service)


Unresolved merge conflict markers are present inside the SQL string in the RETURNING clause. This will send invalid SQL to the database and fail at runtime. Remove the conflict markers and keep a single consistent RETURNING list.

Suggested change

<<<<<<< HEAD

currency, current_balance, available_balance, credit_limit, status,

is_manual, color, notes, created_at, updated_at

=======

currency, current_balance, available_balance, credit_limit,

status, is_manual, color, notes, created_at, updated_at

>>>>>>> 46ef8086 (api: unify Decimal mapping in accounts handler; fix clippy in metrics and currency_service)

currency, current_balance, available_balance, credit_limit, status,

is_manual, color, notes, created_at, updated_at

Copilot · 2025-10-15T14:25:27Z

jive-api/src/handlers/accounts.rs

+        color: row.get("color"),
+        icon: row.get("icon"),
+        notes: row.get("notes"),


The SELECT statement above does not include the icon column, but the response attempts to read icon from the row. Either add icon to the SELECT list or set icon to None in the response to avoid a runtime 'no such column' error.

Copilot · 2025-10-15T14:25:27Z

jive-api/src/handlers/accounts.rs

+        color: row.get("color"),
+        icon: row.get("icon"),
+        notes: row.get("notes"),


In create_account, the RETURNING clause does not include icon, but the response reads icon from the returned row. Add icon to the RETURNING columns or set icon to None to prevent a runtime error.

…Decimal dynamic stats)

…Option<DateTime> handling

zensgit added 3 commits October 14, 2025 23:00

api: unify Decimal mapping in accounts handler; fix clippy in metrics…

03c2cdb

… and currency_service

api: fix Decimal/f64 mismatches and Option<DateTime> handling; refact…

dbba450

…or dynamic SQLx reads for accounts + currency/exchange

api/migrations: make net worth tables idempotent; ensure pgcrypto; fi…

a5a63ba

…x IF NOT EXISTS for indexes/triggers

Copilot AI review requested due to automatic review settings October 15, 2025 14:22

gemini-code-assist bot reviewed Oct 15, 2025

View reviewed changes

Copilot AI reviewed Oct 15, 2025

View reviewed changes

zensgit added 2 commits October 15, 2025 22:57

merge: resolve accounts handler conflict on exchange-decimal branch (…

bfd302d

…Decimal dynamic stats)

merge: resolve currency_service & exchange_rate_api conflicts; unify …

7685ad0

…Option<DateTime> handling

zensgit merged commit ed890ae into chore/invitations-audit-align-dev-mock Oct 15, 2025

zensgit deleted the chore/exchange-decimal-migration branch October 15, 2025 15:06

This was referenced Oct 15, 2025

tests: add Decimal serialization contract tests #93

Merged

tests(api): add budget report/progress Decimal serialization contract (ignored until budget Decimal migration) #95

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

api: migrate exchange rate & market to Decimal #92

api: migrate exchange rate & market to Decimal #92

Uh oh!

zensgit commented Oct 15, 2025

Uh oh!

gemini-code-assist bot commented Oct 15, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Oct 15, 2025

Uh oh!

gemini-code-assist bot Oct 15, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 15, 2025

Uh oh!

Copilot AI Oct 15, 2025

Uh oh!

Copilot AI Oct 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

api: migrate exchange rate & market to Decimal #92

api: migrate exchange rate & market to Decimal #92

Uh oh!

Conversation

zensgit commented Oct 15, 2025

Uh oh!

gemini-code-assist bot commented Oct 15, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant