Skip to content

bug fixes#75

Merged
keraliss merged 1 commit into
mainfrom
bug_fixes
Mar 25, 2026
Merged

bug fixes#75
keraliss merged 1 commit into
mainfrom
bug_fixes

Conversation

@keraliss
Copy link
Copy Markdown
Collaborator

@keraliss keraliss commented Mar 24, 2026

Summary by CodeRabbit

  • New Features

    • "Start Your First Swap" empty-state CTA and a "Clear History" action in swap history
    • Improved on-demand swap report loading for viewing individual reports
  • Bug Fixes

    • More robust fee normalization and percentage calculations in swap reports and history
    • Improved maker usability classification for more reliable offers
  • Refactor

    • Reworked swap history data flow and UI rendering
  • Style

    • Removed deprecated CSS utilities and added new minimum-width utilities

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 24, 2026

Warning

Rate limit exceeded

@keraliss has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 44 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f7c86b9d-0946-4ec9-a419-820bfaa5b3e1

📥 Commits

Reviewing files that changed from the base of the PR and between 4e33b97 and 3e932b4.

📒 Files selected for processing (7)
  • CLAUDE.md
  • api1.js
  • src/components/swap/Coinswap.js
  • src/components/swap/SwapHistory.js
  • src/components/swap/SwapReport.js
  • src/components/swap/SwapStateManager.js
  • src/styles/output.css
📝 Walkthrough

Walkthrough

Normalize maker state classification and introduce a reusable isUsableMaker helper; unify and harden fee normalization/derivation across Coinswap, SwapReport, and SwapHistory; add persistent swap history APIs and UI refactor in SwapHistory; remove several unused Tailwind utilities and add min-width utilities.

Changes

Cohort / File(s) Summary
API / Maker categorization
api1.js
Add isUsableMaker(maker) helper that normalizes maker.state (string lowercasing or first object key) and classifies makers into good/unresponsive/bad buckets; replace repeated predicates with makers.filter(isUsableMaker) in taker offer flow.
Fee normalization (runtime + UI)
src/components/swap/Coinswap.js, src/components/swap/SwapReport.js
Introduce toNumber/normalization helpers and new selection logic: derive derivedTotalFee from fee_paid_or_earned (abs when finite) or fallback to totalMakerFees + miningFee; select normalizedTotalFee/totalFee using finiteness/sign checks; recompute miningFee and feePercentage from normalized totals.
Swap history & report UX
src/components/swap/SwapHistory.js, src/components/swap/SwapStateManager.js, src/components/swap/SwapReport.js
Add SwapStateManager.getSwapHistory() and clearSwapHistory(); refactor SwapHistory to use module-level swapHistory, filter for status === 'completed', replace markup builders with buildSwapHistoryList(), add viewSwapReport(swapId) that lazy-loads SwapReport.js, add empty-state CTA and "Clear History" action, and inline header stats.
CSS utilities
src/styles/output.css
Remove numerous unused Tailwind-generated utilities and the --container-3xl variable; add min-w-[180px] and min-w-max utilities.

Sequence Diagrams

sequenceDiagram
    participant User as User
    participant UI as SwapHistoryComponent
    participant SM as SwapStateManager
    participant API as Backend API
    participant SR as SwapReportComponent

    User->>UI: Open Swap History
    UI->>SM: getSwapHistory()
    SM-->>UI: swap_history (filtered completed)
    UI->>User: render list / empty CTA

    User->>UI: Click swap row (swapId)
    UI->>API: fetch swap report (swapId)
    API-->>UI: report data
    UI->>UI: dynamic import SwapReport.js
    UI->>SR: render SwapReportComponent(report)
    SR->>SR: apply fee normalization logic
    SR-->>User: display detailed report
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • App updates #72 — Overlaps swap modules and fee/totalFee normalization; likely related changes to SwapReport/SwapHistory/SwapStateManager and api1.js.

Poem

🐰 I hop through code with careful cheer,
Makers sorted, fees made clear,
Histories saved and views that load,
Clear the past and hit the road—
A tidy burrow, swaps sincere. ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'bug fixes' is vague and generic, failing to convey the specific nature of the changes across multiple files. Use a more descriptive title that captures the primary focus, such as 'Normalize maker state logic and improve fee calculations' or 'Fix maker classification and fee derivation across swap components'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bug_fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/swap/Coinswap.js (1)

726-788: 🧹 Nitpick | 🔵 Trivial

Consider adding a comment explaining the fee derivation logic.

The fee selection logic (preferring fee_paid_or_earned, then falling back to component fees, then validating against provided total_fee) is non-trivial. A brief comment would help future maintainers understand the intent and the relationship between feePaidOrEarned, derivedTotalFee, and normalizedTotalFee. Based on learnings: "Comment complex Bitcoin/swap logic".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/swap/Coinswap.js` around lines 726 - 788, Add a short
explanatory comment above the fee-derivation block (around feePaidOrEarned,
derivedTotalFee, normalizedTotalFee, calculatedMiningFee, and feePercentage)
that summarizes the decision order: prefer explicit fee_paid_or_earned when
present (abs value), otherwise compute derivedTotalFee as sum of maker component
fees (totalMakerFees) plus miningFee, then validate/override with provided
total_fee when it is positive or when derivedTotalFee is non-positive to avoid
understating fees; also note how calculatedMiningFee is computed as miningFee or
the residual (normalizedTotalFee - totalMakerFees) and that feePercentage is
normalizedTotalFee / targetAmount. Reference makerFeeInfo/totalMakerFees and
targetAmount to help future readers locate related variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api1.js`:
- Around line 911-926: The state normalization and categorization logic
(normalizedState, unresponsiveMakers, badMakers, goodMakers) is richer than the
simple checks used later in the coinswap:start goodMakersCount filters, causing
inconsistent usability decisions; consolidate the normalization into/rely on the
existing isUsableMaker(maker) helper (or implement it to return false for
Unresponsive/Bad and true for Good or fallback by checking maker.offer) and
replace the two goodMakersCount filters in coinswap:start to call
isUsableMaker(maker) so both categorization and swap preflight use the same
usable-maker logic.

In `@src/components/swap/Coinswap.js`:
- Around line 777-784: The derivedTotalFee logic in Coinswap.js is excluding
feePaidOrEarned === 0 while SwapReport.js and SwapHistory.js do not, causing
inconsistent totalFee; update the derivedTotalFee calculation by removing the
`feePaidOrEarned !== 0` check so it only tests
`Number.isFinite(feePaidOrEarned)` (keep the fallback to `totalMakerFees +
miningFee`) and ensure normalizedTotalFee logic remains unchanged; adjust the
expression that computes `derivedTotalFee` (referencing variables
derivedTotalFee, feePaidOrEarned, totalMakerFees, miningFee, and
normalizedTotalFee) so all three components use the same finite-only check.

In `@src/components/swap/SwapHistory.js`:
- Around line 1-5: The import list in SwapHistory.js includes an unused symbol
formatElapsedTime; remove formatElapsedTime from the import from
'./SwapStateManager.js' so only SwapStateManager and formatRelativeTime are
imported, ensuring there are no unused imports and avoiding linter errors.
- Around line 145-166: The component contains duplicate helper functions — move
the inner satsToBtc definition out to module scope and reuse the existing
module-level formatDuration and formatDate functions instead of redefining them
inside the component; remove the in-component definitions for satsToBtc,
formatDuration, and formatDate (or keep only a reference) so the component calls
the top-level satsToBtc, formatDuration, and formatDate functions (search for
satsToBtc, formatDuration, formatDate in SwapHistory.js to update usages).
- Around line 171-172: The debug log that prints result.report.report can throw
if result.report is null/undefined; change the console.log to safely access the
nested value (e.g., use optional chaining or a conditional check) such as
console.log('📋 Report from result:', result.report?.report) or wrap with if
(result.report) { console.log(...result.report.report) } so the function won't
crash when result.report is missing; update the console.log in SwapHistory.js
where result.report.report is referenced accordingly.
- Around line 110-126: The inline mapping inside loadSwapHistory bypasses
normalizeSwapReport and causes inconsistent fee/field derivation; update the
mapping to call normalizeSwapReport(report) for each filtered completed report
and use the returned object's fields (e.g., normalized.totalFee,
normalized.totalOutputAmount, normalized.makersCount, normalized.hops,
normalized.feePercentage) when building the swapHistory entry, preserving
id/completedAt/amount fallbacks and computing durationSeconds from
normalized.completedAt and normalized.startedAt (with existing fallback to
Date.now()/0 as needed); ensure you still filter by report.status ===
'completed' before normalizing.
- Around line 372-383: Implement missing async methods on SwapStateManager: add
an async getSwapHistory() that returns state[STORAGE_KEYS.SWAP_HISTORY] || []
and an async clearSwapHistory() that sets state[STORAGE_KEYS.SWAP_HISTORY] = []
(or deletes the key) and persists the state using the same persistence helper
used by other methods in SwapStateManager (follow the pattern of existing async
methods for read/write). Then update the caller in SwapHistory.js to await
SwapStateManager.clearSwapHistory() (replace the direct call at the click
handler) and ensure any other callers use await for these async methods.

In `@src/components/swap/SwapReport.js`:
- Around line 74-81: The current feePercentage expression uses || which treats a
legitimate 0 as falsy and forces recalculation; update the checks to use nullish
coalescing so explicit zeros are preserved: replace uses of
swapReport.feePercentage || swapReport.fee_percentage with
swapReport.feePercentage ?? swapReport.fee_percentage ?? ... and ensure the
fallback still computes (rawTotalFee / (swapReport.targetAmount ??
swapReport.target_amount ?? 0)) * 100 only when targetAmount/target_amount is >
0 to avoid division by zero (refer to feePercentage, swapReport.feePercentage,
swapReport.fee_percentage, rawTotalFee, swapReport.targetAmount,
swapReport.target_amount).

---

Outside diff comments:
In `@src/components/swap/Coinswap.js`:
- Around line 726-788: Add a short explanatory comment above the fee-derivation
block (around feePaidOrEarned, derivedTotalFee, normalizedTotalFee,
calculatedMiningFee, and feePercentage) that summarizes the decision order:
prefer explicit fee_paid_or_earned when present (abs value), otherwise compute
derivedTotalFee as sum of maker component fees (totalMakerFees) plus miningFee,
then validate/override with provided total_fee when it is positive or when
derivedTotalFee is non-positive to avoid understating fees; also note how
calculatedMiningFee is computed as miningFee or the residual (normalizedTotalFee
- totalMakerFees) and that feePercentage is normalizedTotalFee / targetAmount.
Reference makerFeeInfo/totalMakerFees and targetAmount to help future readers
locate related variables.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f7c38bd3-9d92-4708-8c57-f9feb90dd804

📥 Commits

Reviewing files that changed from the base of the PR and between d00c050 and 8f69d66.

📒 Files selected for processing (5)
  • api1.js
  • src/components/swap/Coinswap.js
  • src/components/swap/SwapHistory.js
  • src/components/swap/SwapReport.js
  • src/styles/output.css

Comment thread api1.js
Comment thread src/components/swap/Coinswap.js Outdated
Comment on lines +1 to +5
import {
SwapStateManager,
formatRelativeTime,
formatElapsedTime,
} from './SwapStateManager.js';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Unused import: formatElapsedTime.

formatElapsedTime is imported but never used in this file.

Suggested fix
 import {
   SwapStateManager,
   formatRelativeTime,
-  formatElapsedTime,
 } from './SwapStateManager.js';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import {
SwapStateManager,
formatRelativeTime,
formatElapsedTime,
} from './SwapStateManager.js';
import {
SwapStateManager,
formatRelativeTime,
} from './SwapStateManager.js';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/swap/SwapHistory.js` around lines 1 - 5, The import list in
SwapHistory.js includes an unused symbol formatElapsedTime; remove
formatElapsedTime from the import from './SwapStateManager.js' so only
SwapStateManager and formatRelativeTime are imported, ensuring there are no
unused imports and avoiding linter errors.

Comment thread src/components/swap/SwapHistory.js Outdated
Comment on lines +145 to +166
function satsToBtc(sats) {
if (typeof sats !== 'number' || isNaN(sats)) return '0.00000000';
return (sats / 100000000).toFixed(8);
}

return `
<div class="overflow-hidden rounded-lg border border-gray-800">
<div class="grid grid-cols-[1.7fr_0.9fr_1fr_1fr_1fr_1.25fr] gap-4 bg-[#0f1419] p-4 text-xs font-semibold text-gray-400 uppercase tracking-wide">
<div>Swap</div>
<div>Status</div>
<div>Protocol</div>
<div>Amount</div>
<div>Fee Paid</div>
<div>When</div>
</div>
<div class="divide-y divide-gray-800">
function formatDuration(seconds) {
if (typeof seconds !== 'number' || isNaN(seconds)) return '0m 0s';
const mins = Math.floor(seconds / 60);
const secs = Math.floor(seconds % 60);
return `${mins}m ${secs}s`;
}

function formatDate(timestamp) {
const date = new Date(timestamp);
return date.toLocaleDateString('en-US', {
year: 'numeric',
month: 'short',
day: 'numeric',
hour: '2-digit',
minute: '2-digit',
});
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Duplicate function definitions inside component.

satsToBtc, formatDuration, and formatDate are defined inside the component, but formatDuration (lines 9-14) and formatDate (lines 16-25) already exist at module scope. Consider reusing the module-level functions and extracting satsToBtc to module scope as well.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/swap/SwapHistory.js` around lines 145 - 166, The component
contains duplicate helper functions — move the inner satsToBtc definition out to
module scope and reuse the existing module-level formatDuration and formatDate
functions instead of redefining them inside the component; remove the
in-component definitions for satsToBtc, formatDuration, and formatDate (or keep
only a reference) so the component calls the top-level satsToBtc,
formatDuration, and formatDate functions (search for satsToBtc, formatDuration,
formatDate in SwapHistory.js to update usages).

Comment thread src/components/swap/SwapHistory.js
Comment thread src/components/swap/SwapHistory.js Outdated
Comment on lines +372 to +383
content.querySelector('#clear-history')?.addEventListener('click', () => {
if (
confirm(
'Are you sure you want to clear all swap history? This cannot be undone.'
)
) {
SwapStateManager.clearSwapHistory();
// Re-render the component
container.innerHTML = '';
SwapHistoryComponent(container);
}
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if clearSwapHistory is async
ast-grep --pattern 'async clearSwapHistory($$$) { $$$ }'
rg -n 'clearSwapHistory' --type js -A 3

Repository: citadel-tech/taker-app

Length of output: 368


🏁 Script executed:

#!/bin/bash
# Find SwapStateManager.js
fd -n "SwapStateManager\.js" -t f

# Once found, search for the clearSwapHistory method definition
rg -n "clearSwapHistory\s*[(:=]" --type js -B 2 -A 10 | head -50

Repository: citadel-tech/taker-app

Length of output: 1180


🏁 Script executed:

#!/bin/bash
# Find SwapStateManager.js
fd "SwapStateManager\.js" -t f

# Search for clearSwapHistory method definition
rg "clearSwapHistory\s*[(:=]" --type js -B 2 -A 15

Repository: citadel-tech/taker-app

Length of output: 1106


🏁 Script executed:

#!/bin/bash
# Check file size
wc -l src/components/swap/SwapStateManager.js

# Search for clearSwapHistory in the file with broader pattern
rg "clearSwapHistory" src/components/swap/SwapStateManager.js -B 3 -A 10

Repository: citadel-tech/taker-app

Length of output: 109


🏁 Script executed:

#!/bin/bash
# Read the entire SwapStateManager.js file with line numbers
cat -n src/components/swap/SwapStateManager.js

Repository: citadel-tech/taker-app

Length of output: 8431


🏁 Script executed:

#!/bin/bash
# Search for clearSwapHistory definition across all JS files
rg "clearSwapHistory" --type js -n

# Also search for getSwapHistory to see if swap history retrieval is implemented
rg "getSwapHistory" --type js -n

Repository: citadel-tech/taker-app

Length of output: 235


🏁 Script executed:

#!/bin/bash
# Check if getSwapHistory is defined anywhere
rg "getSwapHistory\s*\(" --type js -B 2 -A 10

# Also search for any method that handles clearing history
rg "SWAP_HISTORY" --type js -n -B 2 -A 2

Repository: citadel-tech/taker-app

Length of output: 1937


🏁 Script executed:

#!/bin/bash
# Read SwapHistory.js around the problematic line
sed -n '365,390p' src/components/swap/SwapHistory.js

Repository: citadel-tech/taker-app

Length of output: 855


clearSwapHistory() method is missing from SwapStateManager and needs to be implemented.

SwapStateManager.clearSwapHistory() is called at line 378 but the method doesn't exist in SwapStateManager.js, causing a runtime error. The method should be implemented as async (following the pattern of other methods) to clear state[STORAGE_KEYS.SWAP_HISTORY]. Additionally, the call needs to be awaited since all SwapStateManager methods are asynchronous.

Note: getSwapHistory() referenced at SwapStateManager.js:171 is also missing and should be implemented.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/swap/SwapHistory.js` around lines 372 - 383, Implement missing
async methods on SwapStateManager: add an async getSwapHistory() that returns
state[STORAGE_KEYS.SWAP_HISTORY] || [] and an async clearSwapHistory() that sets
state[STORAGE_KEYS.SWAP_HISTORY] = [] (or deletes the key) and persists the
state using the same persistence helper used by other methods in
SwapStateManager (follow the pattern of existing async methods for read/write).
Then update the caller in SwapHistory.js to await
SwapStateManager.clearSwapHistory() (replace the direct call at the click
handler) and ensure any other callers use await for these async methods.

Comment on lines +74 to +81
feePercentage:
swapReport.feePercentage ||
swapReport.fee_percentage ||
((swapReport.targetAmount ?? swapReport.target_amount ?? 0) > 0
? (rawTotalFee /
(swapReport.targetAmount ?? swapReport.target_amount ?? 0)) *
100
: 0),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Falsy check may incorrectly override a legitimate zero feePercentage.

Using || means a valid feePercentage of 0 from the backend will be ignored and recalculated. Consider using nullish coalescing (??) for the initial checks:

Suggested fix
     feePercentage:
-      swapReport.feePercentage ||
-      swapReport.fee_percentage ||
+      (swapReport.feePercentage ?? swapReport.fee_percentage) ??
       ((swapReport.targetAmount ?? swapReport.target_amount ?? 0) > 0
         ? (rawTotalFee /
             (swapReport.targetAmount ?? swapReport.target_amount ?? 0)) *
           100
         : 0),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
feePercentage:
swapReport.feePercentage ||
swapReport.fee_percentage ||
((swapReport.targetAmount ?? swapReport.target_amount ?? 0) > 0
? (rawTotalFee /
(swapReport.targetAmount ?? swapReport.target_amount ?? 0)) *
100
: 0),
feePercentage:
(swapReport.feePercentage ?? swapReport.fee_percentage) ??
((swapReport.targetAmount ?? swapReport.target_amount ?? 0) > 0
? (rawTotalFee /
(swapReport.targetAmount ?? swapReport.target_amount ?? 0)) *
100
: 0),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/swap/SwapReport.js` around lines 74 - 81, The current
feePercentage expression uses || which treats a legitimate 0 as falsy and forces
recalculation; update the checks to use nullish coalescing so explicit zeros are
preserved: replace uses of swapReport.feePercentage || swapReport.fee_percentage
with swapReport.feePercentage ?? swapReport.fee_percentage ?? ... and ensure the
fallback still computes (rawTotalFee / (swapReport.targetAmount ??
swapReport.target_amount ?? 0)) * 100 only when targetAmount/target_amount is >
0 to avoid division by zero (refer to feePercentage, swapReport.feePercentage,
swapReport.fee_percentage, rawTotalFee, swapReport.targetAmount,
swapReport.target_amount).

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/components/swap/SwapHistory.js (1)

173-174: ⚠️ Potential issue | 🟡 Minor

Potential null dereference when logging result.report.report.

Line 174 accesses result.report.report without optional chaining. If result.report is undefined or null, this will throw before the success check at line 176.

Suggested fix
       const result = await window.api.swapReports.get(swapId);
       console.log('📋 Raw result from API:', result);
-      console.log('📋 Report from result:', result.report.report);
+      console.log('📋 Report from result:', result.report?.report);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/swap/SwapHistory.js` around lines 173 - 174, The added
console.log that references result.report.report can throw if result.report is
null/undefined; update the logging to safely access the nested property (e.g.,
use optional chaining or explicit guard) so it becomes something like logging
result.report?.report or logging a fallback value when result.report is missing;
locate the logs in SwapHistory (around the variable result) and change both
console.log calls to safely handle a missing result.report before dereferencing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api1.js`:
- Around line 40-56: The fallback currently returns maker.offer !== null which
treats undefined as valid; update the fallback in isUsableMaker to only count
real offers by checking for both null and undefined (e.g. use maker.offer !=
null or Boolean(maker.offer)) so missing offers are not considered usable;
adjust the same logic wherever similar checks appear (the other occurrences
referenced in the comment).

In `@src/components/swap/Coinswap.js`:
- Around line 777-787: Add a short clarifying comment above the fee derivation
block explaining the priority and purpose of each computed variable: describe
that derivedTotalFee uses the absolute feePaidOrEarned when finite else falls
back to totalMakerFees + miningFee, that normalizedTotalFee prefers a positive
backend totalFee otherwise uses derivedTotalFee, and that calculatedMiningFee
derives from miningFee or the remainder (normalizedTotalFee - totalMakerFees);
also note feePercentage uses targetAmount to avoid divide-by-zero. Place this
comment near the computed symbols derivedTotalFee, normalizedTotalFee,
calculatedMiningFee, and feePercentage so future maintainers can quickly
understand the fallback chain.

---

Duplicate comments:
In `@src/components/swap/SwapHistory.js`:
- Around line 173-174: The added console.log that references
result.report.report can throw if result.report is null/undefined; update the
logging to safely access the nested property (e.g., use optional chaining or
explicit guard) so it becomes something like logging result.report?.report or
logging a fallback value when result.report is missing; locate the logs in
SwapHistory (around the variable result) and change both console.log calls to
safely handle a missing result.report before dereferencing.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: df158203-272d-4002-b3a6-f7c584f574af

📥 Commits

Reviewing files that changed from the base of the PR and between 8f69d66 and 4e33b97.

📒 Files selected for processing (6)
  • api1.js
  • src/components/swap/Coinswap.js
  • src/components/swap/SwapHistory.js
  • src/components/swap/SwapReport.js
  • src/components/swap/SwapStateManager.js
  • src/styles/output.css

Comment thread api1.js
Comment on lines +777 to +787
const derivedTotalFee = Number.isFinite(feePaidOrEarned)
? Math.abs(feePaidOrEarned)
: totalMakerFees + miningFee;
const normalizedTotalFee =
Number.isFinite(totalFee) && (totalFee > 0 || derivedTotalFee <= 0)
? totalFee
: derivedTotalFee;
const calculatedMiningFee =
miningFee || normalizedTotalFee - totalMakerFees;
const feePercentage =
targetAmount > 0 ? (totalFee / targetAmount) * 100 : 0;
targetAmount > 0 ? (normalizedTotalFee / targetAmount) * 100 : 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding a brief comment explaining the fee derivation priority.

The fee derivation logic correctly implements a fallback chain, but the conditions are non-obvious. A brief comment would help future maintainers understand the priority:

// Fee derivation priority:
// 1. Use fee_paid_or_earned (absolute value) if available
// 2. Fall back to totalMakerFees + miningFee
// 3. Prefer backend total_fee when positive, else use derived value

As per coding guidelines: "Comment complex Bitcoin/swap logic".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/swap/Coinswap.js` around lines 777 - 787, Add a short
clarifying comment above the fee derivation block explaining the priority and
purpose of each computed variable: describe that derivedTotalFee uses the
absolute feePaidOrEarned when finite else falls back to totalMakerFees +
miningFee, that normalizedTotalFee prefers a positive backend totalFee otherwise
uses derivedTotalFee, and that calculatedMiningFee derives from miningFee or the
remainder (normalizedTotalFee - totalMakerFees); also note feePercentage uses
targetAmount to avoid divide-by-zero. Place this comment near the computed
symbols derivedTotalFee, normalizedTotalFee, calculatedMiningFee, and
feePercentage so future maintainers can quickly understand the fallback chain.

@keraliss keraliss merged commit 74906af into main Mar 25, 2026
1 check passed
@coderabbitai coderabbitai Bot mentioned this pull request Apr 3, 2026
@coderabbitai coderabbitai Bot mentioned this pull request Apr 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant