From 3c798cee8819c163eb6442e305c01995b1b2e48f Mon Sep 17 00:00:00 2001 From: Keith Solomon Date: Fri, 7 Aug 2026 17:30:33 -0500 Subject: [PATCH] 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 --- ...026-08-07-revert-vault-balance-approach.md | 306 ++++++++++++++++++ 1 file changed, 306 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-07-revert-vault-balance-approach.md diff --git a/docs/superpowers/plans/2026-08-07-revert-vault-balance-approach.md b/docs/superpowers/plans/2026-08-07-revert-vault-balance-approach.md new file mode 100644 index 0000000..2e8c17f --- /dev/null +++ b/docs/superpowers/plans/2026-08-07-revert-vault-balance-approach.md @@ -0,0 +1,306 @@ +# 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 ` 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 "

|

|

" | 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. \ No newline at end of file