Skip to content

feat(cli): lead wallets get with balance and activity on a terminal - #108

Merged
thebrandonlucas merged 1 commit into
feat/cli-look-08-contextfrom
feat/cli-look-09-balance
Oct 23, 2026
Merged

thebrandonlucas merged 1 commit into
feat/cli-look-08-contextfrom
feat/cli-look-09-balance

Conversation

@thebrandonlucas

@thebrandonlucas thebrandonlucas commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Stack position: 9 of 13 in the look-and-feel stack. Base: feat/cli-look-08-context (#107).

Problem

voltage wallets get answers "how much is in this wallet?", but the balance was a JSON blob in the middle of 20 fields, in msats, and nothing showed recent activity.

Behavior

On a terminal (stdout is a TTY, table output), the wallet's fields are led by:

Treasury                                              ← Voltage orange
Balance   150,000 sats  ≈ $90.00                      ← amount bold; ≈ USD on mainnet only
Activity  ▁▁▁▁▁▁▁▁▁▃▁▁█▁  6,000 sats completed in 14 days

active               true
balances             [...]
…
Line Rule
Name The wallet's name, in Voltage orange.
Balance One line per balance, showing its available amount in the friendly units from #101; negative balances (a line of credit) keep their sign.
≈ USD Attached to the BTC balance it priced, wherever that is in the list. Only for a BTC balance on mainnet, priced by the price service (--price-url). Test-network coins have no price, so mutinynet and signet show none.
Activity Completed payment volume per day for the last 14 days (today last), summed from each payment's data.amount in the wallet's currency, from its 100 newest completed payments (statuses[]=completed, sort_order=DESC, by cursor). Each bar is scaled to the busiest day; a day with no volume is the floor bar. ASCII bars without a UTF-8 locale.

The extra reads (the BTC price on mainnet, and payments list --wallet ID --limit 100) are best effort: one that fails leaves its line out and the command still succeeds. They happen only on a terminal with table output. Redirected, --output table | cat, and --json make no extra request and print exactly what they did before.

Design notes

  • balance::Headline is pure: from_wallet, price, activity(page, now), sparkline, and lines. All formatting reuses output::readable_amount (the feat(cli): show friendly amounts and relative times, and color tables #101 amount renderer, now exported) and epoch_seconds.
  • Call::read calls lead_with_balance for OperationId::GetWallet when Output::on_terminal(). That writes the headline through Output::preface, the same color-aware stdout as the table, then the usual write_pages. Call now carries &GlobalFlags for the price URL and timeout.
  • Line::styled_brand paints scrubbed text, the wallet's name, in the brand color.
  • USD uses UsdPerBtc::msats_to_cents, the same exact, half-up conversion as voltage convert.

Try it on staging

Setup: Try a pull request on staging (added by #97). Read-only. Find a wallet ID with voltage wallets list --account me.

voltage wallets get WALLET_ID --account me

Expect: the wallet's name in orange, Balance with its available amount (staging wallets are on mutinynet, so there's no ≈ $), and an Activity sparkline with a caption. The usual fields follow after a blank line.

LANG=C LC_ALL=C voltage wallets get WALLET_ID --account me
voltage wallets get WALLET_ID --account me --output table | cat
voltage wallets get WALLET_ID --account me --json | head -c 80

Expect: ASCII bars (_ for empty days). Then the plain fields only, with no headline and no extra requests. Then JSON, unchanged.

Validation

  • cargo fmt --check, cargo clippy --locked --all-targets -- -D warnings, cargo machete --with-metadata, cargo test --locked (164 unit and 128 integration tests), and python3 scripts/check-coverage.py all pass.
  • Unit tests:
    • USD estimate on mainnet only (a negative BTC balance is priced to -$90.00 at $60,000), a USD balance, and an empty wallet.
    • Day bucketing at both window edges, a future date, a non-completed status, and an unreadable time.
    • Sparkline scaling and the floor bar, ASCII bars, and both captions.
  • New integration test, with stdin, stdout, and stderr all on a PTY:
    • Mainnet: the exact headline with ≈ $90.00 from a mocked price service, and the activity read sent with wallet_id and limit=100.
    • Signet: no ≈. A 500 from the activity read leaves the Activity line out, and the command still succeeds.
  • The test pins LANG, removes LC_ALL and LC_CTYPE, and passes under LC_ALL=C. That is how the CRAP run first caught it leaking the runner's locale.
  • Checked by hand on staging (a staging wallet).
  • CRAP: 0 of 743 measured functions at or above 20. The highest is 18.00 (src/cli.rs endpoint, untouched). lead_with_balance is 5.0, Call::read 11.0, and balance.rs at most 6.0.

Docs

docs/guides/wallets.md and CHANGELOG.md.

@thebrandonlucas
thebrandonlucas added this pull request to stack #92 October 22, 2026 22:49
@thebrandonlucas
thebrandonlucas marked this pull request as ready for review October 22, 2026 22:51

@tee8z tee8z left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes on 35e46c0 (base #107 799b6db). The headline appears only with table output to a terminal, with no --output-file, so JSON, NDJSON, piped and file output send no extra requests and print nothing new. The wallet name is scrubbed, the price service gets no credential, and a failed extra read only leaves its line out.

P2: the activity line misses most payment types and can say no payments were made. activity (src/balance.rs:142) sums data.amount_msats. In api/openapi.json, that field is deprecated ("Use amount instead. This field will be removed"). On-chain payments carry amount_sats instead, and other types carry only amount. Their completed payments count as 0, so a wallet whose recent completed payments are all on-chain shows no completed payments in 14 days, which is false. Once amount_msats is removed, every wallet shows that. Read data.amount (an Amount) and sum it per currency. The test fixtures use only amount_msats, which is why the suite misses this.

P2: the USD estimate can attach to the wrong balance. priceable_msats is the first BTC balance anywhere in the list (:46), but the ≈ is added to the balance at index == 0 (:78). With balances [usd $12.34, btc 150,000 sats] on mainnet, the line reads Balance $12.34 ≈ $90.00. Attach the estimate to the BTC balance it was computed from.

P2: the payments read is built by hand. The query at src/api.rs:924 sends only wallet_id and limit:

  • It sends no statuses, so failed and expired payments use up the 100 slots.
  • It sends no sort order, so "newest first" rests on an unstated default.
  • It sends no pagination=cursor, which every other list request gets from input, so the server runs the total-count query that the contract warns is expensive.

When the 100 newest payments cover only a few days, the rest of the 14 days shows as empty, and the caption understates the volume without saying so.

P2 (AGENTS.md: test every new error path): nothing tests the price read failing (an HTTP error, or a price that UsdPerBtc::parse rejects). Nothing asserts that redirected or --json output makes no price or payments request either. An .expect(0) route on a redirected run would catch that.

Nonblocking:

  • The price and payments reads each get the full --timeout (60 seconds by default), and they run one after the other before anything is printed. An unreachable price service delays wallets get by a minute. Bounding them by the command's deadline, or a short limit, would help.
  • The Balance label shows available, not total, without saying so.
  • ≈ ignores the ASCII glyph setting.
  • readable_amount inherits #101's mixed units, so stacked lines can read 150,000 sats above 1,500,001 msats completed.
  • The description names a staging organization. A placeholder would be better.

I have not run the gate or the CRAP report on this head yet. CI is green on it.

Comment thread src/balance.rs Outdated
Comment thread src/balance.rs Outdated
Comment thread src/api.rs Outdated
On a terminal, voltage wallets get first shows the wallet's name in Voltage
orange, each available balance in bold (with a USD estimate for a BTC
balance on mainnet, from the price service), and a sparkline of its
completed payment volume over the last 14 days from its 100 newest
payments. The two extra reads are best effort: a failure leaves its line
out. The wallet's fields follow unchanged, and redirected or JSON output
makes no extra request.
@thebrandonlucas
thebrandonlucas force-pushed the feat/cli-look-09-balance branch from a329faf to 73d4b3f Compare October 23, 2026 12:38
@thebrandonlucas

Copy link
Copy Markdown
Contributor Author

Review-body items, in 73d4b3f:

  • P2, error-path tests:
    • a_failed_or_unreadable_price_leaves_the_estimate_out covers a price service 500 and a price UsdPerBtc::parse rejects: no ≈ either time, and the balance still shows.
    • a_redirected_wallet_read_makes_no_extra_request runs wallets get with --json and piped --output table against price and payments routes set to .expect(0).
  • Timeouts: each extra read has balance::READ_LIMIT (5 s) at most. The price service takes the smaller of that and --timeout, and the payments read is wrapped in tokio::time::timeout.
  • Balance vs total: the label is now Available.
  • ≈ in ASCII: ~.
  • Mixed units: gone with feat(cli): show friendly amounts and relative times, and color tables #101's change; every BTC amount reads in sats.
  • Payment checks: the activity request also sends check_only=false from fix(api): refresh payment and ledger query filters #102's contract.
  • Staging names in the description: replaced with placeholders.

@tee8z tee8z left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved 73d4b3f against feat/cli-look-08-context (#107 d5bd5fa). My findings are fixed:

  • Activity sums data.amount in the wallet's currency through base_units, so on-chain and other payment types count.
  • The USD estimate stays on the BTC balance it priced, and a test covers a USD balance that comes first.
  • The payments read goes through Request::scoped with statuses[]=completed, sort_order=DESC and pagination=cursor, and payment checks are excluded.
  • New tests cover a failed or unreadable price, which leaves the estimate out, and a redirected wallets get, which makes no extra request (.expect(0)).
  • Each extra read is now bounded by a 5-second READ_LIMIT, and the balance line is labelled Available.

Validation on this head, in nix develop: cargo fmt --check, cargo clippy --locked --all-targets -- -D warnings, cargo machete --with-metadata, cargo test --locked (180 unit, 137 integration) and python3 scripts/check-coverage.py passed. CRAP: 779 measured functions, none at or above 20, highest wait_for_payment at 17.05. CI is green.

@thebrandonlucas
thebrandonlucas merged commit 9daf366 into master Oct 23, 2026
9 of 14 checks passed
@thebrandonlucas
thebrandonlucas deleted the feat/cli-look-09-balance branch October 23, 2026 15:47
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.

2 participants