Files
Torn-Vault-Tracker/docs/superpowers/plans/2026-08-07-revert-vault-balance-approach.md
T
Keith Solomon 3c798cee88 Add plan: revert to per-user balance via /log + per-entry re-fetch
- 5 tasks: revert vaultLoop to SQL, replace broadcast backfill with
  per-entry re-fetch, wire into entry points, delete unused helper
  and its test, smoke check
- refetchRunningBalances walks /log pagination and UPDATEs each row
  from its own data.balance
- Drops the fetchLiveVaultBalance helper (queried /money which is
  total-vault not per-user)
- The previous fix's broadcast backfill was clobbering per-entry
  historical balances with a single value
2026-08-07 17:30:33 -05:00

306 lines
11 KiB
Markdown

# Revert to Per-User Balance via `/log` + Per-Entry Re-fetch — Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Correctly compute per-user vault share. The `/money` endpoint returns the *total* vault across all users — wrong for per-user attribution. Revert `vaultLoop` to per-user SQL `MAX(running_balance)`. Replace the broken broadcast backfill with a true per-entry re-fetch that updates each row's `running_balance` from its own `data.balance`. Delete the unused `fetchLiveVaultBalance` helper and its test.
**Architecture:** `vaultLoop` returns `MAX(running_balance)` per user (where each row's `running_balance` comes from that entry's own `data.balance` in the v2 `/log` response). The schema's `running_balance` column is the source of truth. A new `refetchRunningBalances` helper re-fetches each user's full `/log` and updates every row's `running_balance` from its per-entry `data.balance`. The `fetchLiveVaultBalance` helper (introduced in the previous plan) is deleted along with its test, since it queried the wrong endpoint.
**Tech Stack:** PHP 8.1+, SQLite3 via PDO, cURL. No new dependencies.
## Global Constraints
- PHP 8.1+ syntax only.
- Database schema (`vault` table with `id TEXT PRIMARY KEY` and `running_balance INTEGER`) is unchanged.
- `INSERT … ON CONFLICT(id) DO NOTHING` is the only INSERT form used for log entries.
- API authentication: `Authorization: ApiKey <key>` header.
- All helpers outside the rewritten ones stay byte-identical unless the task explicitly says otherwise.
- Commits per task. Do not push.
---
### Task 1: Revert `vaultLoop` to per-user SQL
**Files:**
- Modify: `includes/utilities.php` (`vaultLoop` only)
**Interfaces:**
- `vaultLoop($name=null)`: returns the per-user vault share (per the `running_balance` column). All-users case sums per-user MAXes.
- [ ] **Step 1: Replace `vaultLoop` with the SQL-based implementation**
In `includes/utilities.php`, replace the existing `vaultLoop` function (which currently calls `fetchLiveVaultBalance`) with:
```php
function vaultLoop ($name=null) {
$pdo = getDatabaseConnection();
if ($name === null) {
$stmt = $pdo->query(
'SELECT COALESCE(SUM(max_balance), 0) FROM '
. '(SELECT MAX(running_balance) AS max_balance FROM vault GROUP BY user)'
);
return (int)$stmt->fetchColumn();
}
$stmt = $pdo->prepare('SELECT MAX(running_balance) FROM vault WHERE user = :user');
$stmt->bindValue(':user', $name);
$stmt->execute();
return (int)$stmt->fetchColumn();
}
```
This is the same SQL-based implementation from the previous balance-fix plan, before `/money` was added.
- [ ] **Step 2: Verify the file parses**
Run: `php -l includes/utilities.php`
Expected: `No syntax errors detected`.
- [ ] **Step 3: Run the existing golden-file test (no regression)**
Run: `php tests/process_log_entries_test.php`
Expected: `OK: processLogEntries v2 golden-file test passed (100 entries).`
(The `live_balance_test.php` is expected to FAIL after this commit because `fetchLiveVaultBalance` is now unused but still defined. That's OK — we'll delete the test in Task 4. The golden-file test is the load-bearing one.)
- [ ] **Step 4: Commit**
```bash
git add includes/utilities.php
git commit -m "Revert vaultLoop to per-user SQL MAX(running_balance)"
```
---
### Task 2: Replace `backfillRunningBalances` with `refetchRunningBalances`
**Files:**
- Modify: `includes/utilities.php` (replace the existing function body)
**Interfaces:**
- `refetchRunningBalances(PDO $pdo, string $user): void`
- Counts rows for the user where `running_balance IS NULL`. If 0, returns.
- Otherwise, fetches the user's full `/log` history (paginating with `_metadata.links.next` until null), and for each entry runs an `UPDATE vault SET running_balance = X WHERE id = Y` to set the per-entry value from `data.balance`. Each fetched entry has its own historical balance, which is the correct semantics.
- Calls `executeApiCall` and `validateApiResponse` (existing helpers).
- On API failure, throws the underlying exception (caller catches via try/catch in `index.php`).
- [ ] **Step 1: Replace the function**
In `includes/utilities.php`, find the existing `backfillRunningBalances` function and replace it with:
```php
/**
* Re-fetch the running_balance for each entry of a user by walking the
* v2 /log paginated endpoint.
*
* For each entry the API returns, we run a single UPDATE setting that
* entry's `running_balance` to its `data.balance`. The pre-existing
* broadcast approach gave every row of a user the same value, which
* made the historical view misleading; this restores per-entry accuracy.
*
* If the user has zero rows with `running_balance IS NULL`, returns
* without making any HTTP call. On API failure, the underlying
* exception propagates to the caller's try/catch in index.php.
*
* @param PDO $pdo Database connection.
* @param string $user The user whose NULL rows should be refilled.
*
* @return void
*/
function refetchRunningBalances($pdo, $user) {
if (!array_key_exists($user, USER_KEYS)) {
throw new ApiKeyMissingException("User does not have an API key configured.");
}
$countStmt = $pdo->prepare('SELECT COUNT(*) FROM vault WHERE user = :user AND running_balance IS NULL');
$countStmt->bindValue(':user', $user);
$countStmt->execute();
if ((int)$countStmt->fetchColumn() === 0) {
return;
}
$updateStmt = $pdo->prepare(
'UPDATE vault SET running_balance = :balance WHERE id = :id'
);
$url = 'https://api.torn.com/v2/user?selections=log&log=5850,5851';
do {
$responseData = executeApiCall($url, USER_KEYS[$user]);
validateApiResponse($responseData);
foreach ($responseData['log'] as $entry) {
$id = $entry['id'] ?? null;
if (!$id || !isset($entry['data']['balance'])) {
continue;
}
$updateStmt->bindValue(':balance', (int)$entry['data']['balance']);
$updateStmt->bindValue(':id', $id);
$updateStmt->execute();
}
$url = $responseData['_metadata']['links']['next'] ?? null;
} while ($url !== null);
}
```
- [ ] **Step 2: Verify the file parses**
Run: `php -l includes/utilities.php`
Expected: `No syntax errors detected`.
- [ ] **Step 3: Run the golden-file test (no regression)**
Run: `php tests/process_log_entries_test.php`
Expected: `OK: processLogEntries v2 golden-file test passed (100 entries).`
- [ ] **Step 4: Commit**
```bash
git add includes/utilities.php
git commit -m "Replace broadcast backfill with per-entry re-fetch for running_balance"
```
---
### Task 3: Wire `refetchRunningBalances` into entry points
**Files:**
- Modify: `functions.php` (`backfillUserLogs` and `syncUserLogs`)
**Interfaces:**
- Same as before, but the post-pagination call is now to `refetchRunningBalances` (not `backfillRunningBalances`).
- [ ] **Step 1: Update `backfillUserLogs`**
In `functions.php`, replace the last line of `backfillUserLogs` (`backfillRunningBalances($pdo, $user);`) with `refetchRunningBalances($pdo, $user);`.
- [ ] **Step 2: Update `syncUserLogs`**
Same edit in `syncUserLogs`.
- [ ] **Step 3: Verify the file parses**
Run: `php -l functions.php`
Expected: `No syntax errors detected`.
- [ ] **Step 4: Commit**
```bash
git add functions.php
git commit -m "Invoke refetchRunningBalances after sync/backfill"
```
---
### Task 4: Delete unused `fetchLiveVaultBalance` and its test
**Files:**
- Modify: `includes/utilities.php` (delete the helper function and the test-hook shim line at top of file)
- Delete: `tests/live_balance_test.php`
**Interfaces:**
- `fetchLiveVaultBalance` is removed entirely. Any callers (none after Task 1's revert) won't find it.
- The `$GLOBALS['liveBalanceTestHook']` shim line is removed.
- [ ] **Step 1: Delete the test hook shim**
In `includes/utilities.php`, near the top of the file (right after the doc-block and before `function dbNew`), find and delete the line:
```php
// Test hook for fetchLiveVaultBalance(). When set, this closure is called
// instead of executeApiCall. Set $GLOBALS['liveBalanceTestHook'] to a
// closure($url, $apiKey): array in test code. Production code leaves it null.
$GLOBALS['liveBalanceTestHook'] = $GLOBALS['liveBalanceTestHook'] ?? null;
```
- [ ] **Step 2: Delete the `fetchLiveVaultBalance` function**
Find and delete the entire `fetchLiveVaultBalance` function (including its docblock) at the end of `includes/utilities.php`.
- [ ] **Step 3: Delete the test file**
```bash
git rm tests/live_balance_test.php
```
- [ ] **Step 4: Verify the file parses**
Run: `php -l includes/utilities.php`
Expected: `No syntax errors detected`.
- [ ] **Step 5: Run the golden-file test (no regression)**
Run: `php tests/process_log_entries_test.php`
Expected: `OK: processLogEntries v2 golden-file test passed (100 entries).`
- [ ] **Step 6: Verify the test file is gone**
Run: `ls tests/`
Expected: only `process_log_entries_test.php`.
- [ ] **Step 7: Commit**
```bash
git add includes/utilities.php tests/live_balance_test.php
git commit -m "Remove unused fetchLiveVaultBalance and its test"
```
---
### Task 5: End-to-end smoke check
**Files:** No code changes. Verification only.
- [ ] **Step 1: Run the test suite**
Run: `php tests/process_log_entries_test.php`
Expected: `OK: processLogEntries v2 golden-file test passed (100 entries).`
- [ ] **Step 2: Probe the test instance**
```
curl -s http://127.0.0.1:8001/ | grep -E "<h2>|<h3>|<h4>" | head -15
```
Expected: per-user balances come from `MAX(running_balance)` per user (which after the re-fetch will be the historical entry's actual `data.balance`, not the broadcast value). For Zarathos whose log last saw data.balance=$69,235,831, that's what shows. For Symos whose most recent entry has data.balance=$160,627,669 (or whatever the latest API returns), that's what shows. Both values reflect the per-entry historical balance, not the broadcast approximation.
- [ ] **Step 3: Verify no v1 references remain**
```bash
cd "C:\Users\ksolo\Projects\Games\Torn\Torn Vault Tracker"
grep -n "firstRun\|?key=\|fetchLiveVaultBalance" functions.php index.php includes/utilities.php includes/exceptions.php tests/
```
Expected: no matches (including no references to the deleted `fetchLiveVaultBalance`).
- [ ] **Step 4: Commit if any incidental fixes were needed**
If Step 2 or 3 produced any required fixes, commit them. Otherwise, no commit.
---
## Self-Review Notes
**Spec coverage:**
| Requirement | Task |
|---|---|
| Per-user balance via SQL `MAX(running_balance)` | Task 1 |
| Replace broadcast backfill with per-entry re-fetch | Task 2 |
| Wire re-fetch into entry points | Task 3 |
| Delete unused `fetchLiveVaultBalance` and its test | Task 4 |
| End-to-end smoke check | Task 5 |
**Placeholder scan:** No "TODO", "TBD", "implement later".
**Type consistency:** `refetchRunningBalances(PDO $pdo, string $user): void` matches the call sites in Task 3. The `vaultLoop` SQL matches the documented contract.
**Risks addressed:**
- The broadcast-backfill defect is fixed by per-entry re-fetch.
- Unused code is removed (dead-weight cleanup).
- The `vaultLoop` revert doesn't introduce regressions because the existing golden-file test still passes.