Conversation
- Add 25 screen components: CRUD (add/edit/delete/search/reports), management (tags/trials/bulk/usage), system (config/export/import/backup/restore/help) - Wire all 24 screens into app.tsx CurrentScreen router - All 309 tests pass, build: 3 chunks, 272 kB
- Add tui.tsx entry point for Ink app rendering - Register tuiCommand via gunshi in index.ts - Add ink v7, @inkjs/ui v2, react v19 to dependencies - Update CLAUDE.md with TUI architecture docs
|
Warning Review limit reached
More reviews will be available in 20 minutes and 33 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughAdds a new Ink-based TUI command with shared state, navigation, and screen routing. It also adds subscription, trial, reporting, config, import/export, backup/restore, and help screens, plus related CLI, dependency, and documentation updates. ChangesTUI rollout
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (21)
subtrack/src/tui/screens/restore.tsx-28-37 (1)
28-37: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRoute encrypted restores through
restoreDb()
The encrypted branch only writessubtrack.dbon disk; it never replaces the live_dbinstance. In thissql.jsflow, the UI can keep serving the old in-memory state, and a later save can overwrite the restored file with stale data. Use the same restore path as the plain-backup branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/restore.tsx` around lines 28 - 37, The encrypted restore branch in restoreDb flow is bypassing the normal database reload path and only writing subtrack.db directly. Update the restore screen logic in restore.tsx so the decrypted bytes are passed through restoreDb() just like the plain-backup branch, ensuring the live _db instance is replaced and the UI state reflects the restored data. Keep the encryption-key check and decryption step, but replace the direct writeFileSync-to-db-dir behavior with the same restore path used by the non-encrypted case.Source: Learnings
subtrack/src/tui/app.tsx-119-124 (1)
119-124: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSuspend the global keymap on screens with local inputs.
RestoreScreenandTagManageScreenboth render@inkjs/uiinputs/selects and handle input locally, butKeyboardHandleronly opts out foradd,edit, anddelete. That leaves app-wide navigation/quit keys active while editing those screens.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/app.tsx` around lines 119 - 124, Suspend the global keymap in KeyboardHandler for any screen that has local input handling, not just add/edit/delete. Update the isFormScreen-style guard inside useInput in app.tsx to also exclude RestoreScreen and TagManageScreen (and any similar local-input screens) so their `@inkjs/ui` controls can receive keys without app-wide navigation or quit shortcuts firing.subtrack/src/tui/screens/payment.tsx-12-12 (1)
12-12: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFilter to
activesubscriptions here.
status !== "cancelled"still countspausedrows, so the "No active subscriptions" check and both totals are wrong as soon as paused subscriptions exist.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/payment.tsx` at line 12, The subscriptions filter in the payment screen is too broad because `active` currently includes `paused` rows, which breaks the “No active subscriptions” state and the totals. Update the `active` calculation in `payment.tsx` to use only truly active subscriptions, and keep the downstream `active.length`, totals, and related rendering logic in sync with that narrower definition.subtrack/src/tui/screens/forecast.tsx-7-7 (1)
7-7: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFilter to
activesubscriptions here.
status !== "cancelled"still includespaused, so this "active" forecast will overstate both monthly and yearly totals whenever a paused subscription exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/forecast.tsx` at line 7, The `active` subscription list in `forecast.tsx` is too broad because `subs.filter((s) => s.status !== "cancelled")` still includes paused items and inflates totals. Update the filtering logic in the `active` assignment to include only truly active subscriptions, using the existing subscription status field and the `active` forecast calculation path so monthly and yearly totals exclude paused records.subtrack/src/tui/screens/forecast.tsx-54-60 (1)
54-60: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winConvert smallest-unit prices before formatting.
This helper passes raw stored integers to
Intl.NumberFormatwith0fraction digits, so1490USD renders as$1,490instead of$14.90. Based on learnings, "Store prices as integers in the smallest currency unit (for example, JPY without decimals and USD in cents)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/forecast.tsx` around lines 54 - 60, The formatPrice helper is formatting raw stored smallest-unit integers directly, so values like cents are shown as whole currency amounts; update formatPrice to convert the integer price into major units before calling Intl.NumberFormat, while keeping the existing fallback path intact. Use the formatPrice function in forecast.tsx as the single place to apply this conversion so displayed prices match the stored smallest-currency-unit convention.Source: Learnings
subtrack/src/tui/screens/analytics.tsx-32-37 (1)
32-37: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep cycle totals separated by currency.
cycleCostkeys only bysub.cycle, then Line 75 formats that merged number with the first matching currency. Monthly USD and JPY subscriptions will be added together into one bogus total, and the label can even come from a cancelled subscription because it searchessubs, notactiveSubs.Also applies to: 73-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/analytics.tsx` around lines 32 - 37, The cycle totals in analytics are being merged only by sub.cycle, so different currencies get incorrectly combined and later labeled from the wrong subscription source. Update cycleCost in analytics.tsx to group by both cycle and currency (using activeSubs data), and then adjust the rendering logic that formats the totals to read the currency from the same grouped entry rather than searching subs, so each displayed cycle total stays currency-specific.subtrack/src/tui/screens/subscription-form.tsx-104-106 (1)
104-106: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject non-integer prices before saving.
The form says prices are entered in the smallest unit, but Lines 106 and 164 only reject negatives and
NaN. Values like12.34still pass and violate the persisted price contract beforeonSaveis called. Based on learnings, "Store prices as integers in the smallest currency unit (for example, JPY without decimals and USD in cents)."Suggested fix
case "price": if (!data.price.trim()) return "Price is required" - if (isNaN(Number(data.price)) || Number(data.price) < 0) return "Price must be a non-negative number" + if (!Number.isInteger(Number(data.price)) || Number(data.price) < 0) { + return "Price must be a non-negative integer" + } break @@ const handleConfirm = useCallback(() => { const price = Number(data.price) - if (isNaN(price) || price < 0) { + if (!Number.isInteger(price) || price < 0) { setError("Invalid price") return }Also applies to: 163-165
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/subscription-form.tsx` around lines 104 - 106, The price validation in the subscription form still allows decimal values, which breaks the smallest-unit integer price contract before onSave runs. Update the price checks in the validation logic for the price field (the switch case handling "price" and the related validation around the save path) so they reject any non-integer input in addition to empty, NaN, or negative values. Use the existing form validation helpers in subscription-form.tsx to enforce that only whole numbers are accepted for price.Source: Learnings
subtrack/src/tui/screens/analytics.tsx-18-18 (1)
18-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFilter to
activesubscriptions here.
status !== "cancelled"still includespaused, so the derived analytics are not actually limited to active subscriptions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/analytics.tsx` at line 18, The active subscription filter in analytics.tsx is too broad because the current filter in the activeSubs derivation includes paused subscriptions; update the logic to only include subscriptions whose status is actually active, using the activeSubs variable as the target to keep the analytics limited to active subscriptions.subtrack/package.json-47-47 (1)
47-47: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDon't mask real
typosfailures.
typos || echo ...treats "found spelling errors" the same as "command not found", so this script now exits successfully even whentyposis installed and reports problems.Suggested fix
- "lint:typos": "typos || echo 'typos not installed, skipping'", + "lint:typos": "command -v typos >/dev/null 2>&1 && typos || { code=$?; [ \"$code\" -eq 127 ] && echo 'typos not installed, skipping' || exit \"$code\"; }",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/package.json` at line 47, The lint:typos script is masking real typos failures by turning any non-zero exit from typos into success. Update the package.json script so it only skips when the typos command is unavailable, while still propagating actual spelling errors reported by typos; use the lint:typos entry in package.json as the place to adjust this behavior.subtrack/src/tui/screens/import.tsx-26-40 (1)
26-40: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftWrap the CSV import in one transaction.
This loop persists rows one by one, so any failure partway through leaves a partial import committed. Based on learnings, use
sql.jswithPRAGMA foreign_keys = ON, and use transactions for multi-step writes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/import.tsx` around lines 26 - 40, The CSV import logic in the import screen currently writes each row independently via writeSubscription, which can leave a partial import committed if a later row fails. Update the import flow around the loop in the import screen to run the entire batch inside a single sql.js transaction, and enable PRAGMA foreign_keys = ON before performing the multi-row write so all inserts either succeed together or roll back together.Source: Learnings
subtrack/src/tui/screens/import.tsx-30-38 (1)
30-38: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate and normalize
fields[3]before writing.
Number(fields[3])turns blank values into0, invalid text intoNaN, and human amounts like9.99into a non-integer stored price. Based on learnings, store prices as integers in the smallest currency unit (for example, JPY without decimals and USD in cents).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/import.tsx` around lines 30 - 38, Validate and normalize the price field before calling writeSubscription in import.tsx: the current Number(fields[3]) conversion can silently turn blanks into 0, invalid text into NaN, and decimal amounts into the wrong stored value. Update the import parsing logic to reject or sanitize fields[3] and convert it into an integer in the smallest currency unit, using the existing fields[1]/fields[4] validation context so the imported subscription price is stored consistently.Source: Learnings
subtrack/src/tui/screens/bulk.tsx-10-27 (1)
10-27: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftWire
selectedinto an actual selection step.
selectedis only mutated bytoggleSub, buttoggleSubis never used and the render path never shows any subscriptions to pick from. This screen therefore always reaches confirmation with0items, andexecuteAction()no-ops on “Yes”.Also applies to: 62-79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/bulk.tsx` around lines 10 - 27, The bulk screen never presents any subscriptions to choose from, so `selected` stays unused and actions confirm with 0 items. Hook `toggleSub` and `selected` into the `bulk.tsx` render flow by adding a subscription selection step before confirmation, rendering the list from `subs`, allowing items to be toggled, and only enabling `executeAction()` once there is at least one selected subscription. Keep the existing state and callbacks (`selected`, `setSelected`, `toggleSub`, `executeAction`) wired together so the action operates on the chosen items instead of no-opping.subtrack/src/tui/screens/summary.tsx-7-15 (1)
7-15: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winConvert smallest-unit prices before formatting.
monthlyByCurrencyaccumulates stored integer prices, butformatPrice()feeds that raw value straight intoIntl.NumberFormat. For cent-based currencies,999will render as$999instead of$9.99. Based on learnings, store prices as integers in the smallest currency unit (for example, JPY without decimals and USD in cents).Also applies to: 37-37, 75-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/summary.tsx` around lines 7 - 15, The price formatting in formatPrice currently treats stored integer amounts as whole currency units, so monthlyByCurrency totals are displayed incorrectly for smallest-unit values. Update formatPrice (and any call sites feeding it) to convert from the stored smallest unit into display units before passing the value to Intl.NumberFormat, while preserving zero-decimal currencies like JPY; use the existing summary screen helpers and the monthlyByCurrency aggregation points to ensure all displayed totals are normalized consistently.Source: Learnings
subtrack/src/tui/screens/trial-add.tsx-25-25 (1)
25-25: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winNormalize the entered price before
writeTrial().The form accepts a human amount string, but confirmation writes
Number(data.price)directly. That allows invalid values through and stores9.99instead of999for cent-based currencies. Based on learnings, store prices as integers in the smallest currency unit (for example, JPY without decimals and USD in cents).Also applies to: 37-39
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/trial-add.tsx` at line 25, Normalize the entered price in the trial add screen before calling writeTrial in the submit flow, since data.price is currently passed through Number(...) and can store invalid or decimal values directly. Update the handler around writeTrial so it converts the user-facing amount string into an integer in the smallest currency unit, handling currencies like USD as cents and zero-decimal currencies like JPY without fractions. Keep the fix localized to the form submission logic in the trial-add screen and ensure writeTrial always receives the normalized integer value.Source: Learnings
subtrack/src/tui/screens/backup.tsx-21-42 (1)
21-42: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winFail closed on encryption and pass the selected mode into
doBackup().
setEncrypt()is async, sodoBackup()reads the previous value on the same event. On top of that, theelsebranch silently writes a plaintext copy when no encryption key is configured. Selecting “Yes, encrypt” can therefore still produce an unencrypted backup and report success.Suggested fix
- const doBackup = useCallback(() => { + const doBackup = useCallback((encryptBackup: boolean) => { try { saveDb() mkdirSync(dest, { recursive: true }) const ts = new Date().toISOString().replace(/[:.]/g, "-") const destPath = join(dest, `subtrack-${ts}.db`) - if (encrypt && hasEncryptionKey()) { + if (encryptBackup) { + if (!hasEncryptionKey()) { + throw new Error("Encryption key is not configured") + } const dbPath = join(getDbDir(), "subtrack.db") const data = readFileSync(dbPath) const encrypted = encryptBuffer(data) writeFileSync(destPath, encrypted) } else { @@ } catch (e: unknown) { setResult(`Backup failed: ${e instanceof Error ? e.message : String(e)}`) } setStep("done") - }, [dest, encrypt]) + }, [dest]) @@ - <Select options={[{label:"Yes, encrypt",value:"yes"},{label:"No, plain copy",value:"no"}]} onChange={(v) => { setEncrypt(v === "yes"); doBackup() }} /> + <Select options={[{label:"Yes, encrypt",value:"yes"},{label:"No, plain copy",value:"no"}]} onChange={(v) => { const encryptBackup = v === "yes"; setEncrypt(encryptBackup); doBackup(encryptBackup) }} />Also applies to: 55-55
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/backup.tsx` around lines 21 - 42, The backup flow in doBackup currently uses the stale encrypt state and falls back to a plaintext copy when no encryption key is available, so “encrypt” can still save an unencrypted file. Update the backup action so the selected mode is passed explicitly into doBackup from the choice handler, and make doBackup fail closed by refusing to proceed unless encryption is actually requested and hasEncryptionKey() is true. Keep the success path in doBackup limited to the intended mode, and use the existing doBackup, setEncrypt, hasEncryptionKey, and encryptBuffer/copyFileSync paths to locate the fix.subtrack/src/tui/screens/trials.tsx-29-31 (1)
29-31: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRender trial prices from minor units, not the raw stored integer.
If
t.priceis stored in cents, this will display999 USDinstead of9.99 USD. Based on learnings, "Store prices as integers in the smallest currency unit (for example, JPY without decimals and USD in cents)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/trials.tsx` around lines 29 - 31, The trial price rendering in the trials screen is using the raw stored integer instead of converting from minor currency units, so the displayed amount can be misleading. Update the price formatting in the trials view component (the block that renders t.price alongside t.currency and t.cycle) to convert from smallest units to a human-readable major-unit amount before displaying it, while keeping the currency and billing cycle labels unchanged.Source: Learnings
subtrack/src/tui/screens/trials.tsx-19-20 (1)
19-20: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExpired trials within the last 24 hours render as still active.
Math.ceilturns small negative deltas into0, so a trial that expired earlier today shows(0d)instead of(expired). Compare timestamps for expiration separately from the displayed day count.Also applies to: 26-26
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/trials.tsx` around lines 19 - 20, In the trials screen rendering logic, the current daysLeft calculation in the expiration display treats trials that expired earlier today as active because Math.ceil rounds small negative time deltas to 0. Update the expiration handling in the relevant screen/component so the expired state is determined by a direct timestamp comparison (expires.getTime() versus now.getTime()) and only use the rounded day count for display when the trial is not expired; keep the existing color/label logic in sync with that separate expired check.subtrack/src/tui/screens/upcoming.tsx-55-59 (1)
55-59: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFormat stored minor-unit prices before rendering.
This formats the raw stored integer directly, so USD-cent values like
999will render as$999instead of$9.99. Based on learnings, "Store prices as integers in the smallest currency unit (for example, JPY without decimals and USD in cents)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/upcoming.tsx` around lines 55 - 59, The formatPrice helper in upcoming.tsx is rendering stored minor-unit integers directly, so values like cents are shown as whole currency amounts. Update formatPrice to convert the raw stored amount from the smallest currency unit into major units before passing it to Intl.NumberFormat, while keeping the existing currency formatting behavior for different currencies. Use the formatPrice function as the fix point and ensure the Upcoming screen renders the adjusted value.Source: Learnings
subtrack/src/tui/screens/upcoming.tsx-46-51 (1)
46-51: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClamp invalid billing days before constructing the next charge date.
For values like
billingDay = 31in a 30-day month,new Date(year, month, day)rolls over to the next month, so this can report July 1 as the next bill instead of the last valid billing date in June/July.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/upcoming.tsx` around lines 46 - 51, Clamp the billing day before creating the next charge date in computeNextBill so invalid days do not roll into the following month. Update the logic around the Date construction in computeNextBill to cap day at the last valid day for the target month, then continue the existing from/until comparison flow so billingDay values like 31 in shorter months resolve to the correct month-end date.subtrack/src/tui/screens/list.tsx-115-119 (1)
115-119: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClamp or clear the selection when the filtered list shrinks.
If
state.listIndexpoints past the end ofsubs, this effect stops updatingselected_sub_idand leaves the previous record selected in global state. The UI can show no selected row while edit/delete actions still target the old subscription.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/list.tsx` around lines 115 - 119, Clamp or clear the selected subscription when the filtered list shrinks, because the current effect in list.tsx only updates selected_sub_id when state.listIndex is still within subs and otherwise leaves stale global selection behind. Update the useEffect tied to state.listIndex and subs so it handles out-of-range indices by either clamping to the last valid item or dispatching a clear/reset action, while keeping the existing SET_SELECTED_SUB_ID path for valid subs[state.listIndex].id values.subtrack/src/tui/screens/list.tsx-18-29 (1)
18-29: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winConvert minor-unit prices before formatting them.
This helper formats stored integers as if they were already major-unit amounts, so a USD price of
500renders as$500instead of$5.00. The same bug then propagates to the totals here and to the new TUI screens following the same pattern. Based on learnings, prices are stored as integers in the smallest currency unit.💱 Suggested fix
-function formatPrice(price: number, currency: string): string { +function formatPrice(priceMinor: number, currency: string): string { try { - return new Intl.NumberFormat("en-US", { - style: "currency", - currency, - minimumFractionDigits: 0, - maximumFractionDigits: 0, - }).format(price) + const formatter = new Intl.NumberFormat("en-US", { + style: "currency", + currency, + }) + const fractionDigits = formatter.resolvedOptions().maximumFractionDigits + return formatter.format(priceMinor / 10 ** fractionDigits) } catch { - return `${currency} ${price}` + return `${currency} ${priceMinor}` } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/list.tsx` around lines 18 - 29, The formatPrice helper is treating stored minor-unit integers as major-unit amounts, so values like 500 render incorrectly in the TUI screens. Update formatPrice in list.tsx to convert the integer price into major units before passing it to Intl.NumberFormat, and apply the same conversion anywhere the totals are computed or displayed in the new TUI screens that follow this pattern. Use the existing formatPrice call sites in the list screen as the reference point and ensure the fallback string also reflects the converted value.Source: Learnings
🟡 Minor comments (7)
subtrack/src/tui/screens/restore.tsx-53-55 (1)
53-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDon't advance into confirmation when there are no backups.
When
optionsis empty, selecting the placeholder still setssourceto""and moves to"confirm", which immediately degrades into a fake restore attempt and a"File not found"error. Hide the selector or block the transition until there is a real path.Suggested change
- <Select - options={options.length > 0 ? options : [{ label: "No backups found", value: "" }]} - onChange={(v) => { setSource(v); setStep("confirm") }} - /> + {options.length > 0 ? ( + <Select + options={options} + onChange={(v) => { setSource(v); setStep("confirm") }} + /> + ) : ( + <Text dimColor>No backups found</Text> + )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/restore.tsx` around lines 53 - 55, The restore screen in restore.tsx should not advance to the confirm step when there are no backups available. Update the Select handling in the restore flow so that the placeholder option cannot trigger setStep("confirm"), and instead hide the selector or guard the onChange path when options is empty. Use the restore screen’s source/state flow and the Select onChange logic to ensure only a real backup path can move to confirmation.subtrack/src/tui/screens/tag-manage.tsx-17-17 (1)
17-17: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRe-query tags after mutations.
useMemo(() => getTagsWithCount(), [])freezes the initial snapshot, so rename/delete/prune returns to list mode with stale names/counts until the screen remounts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/tag-manage.tsx` at line 17, The tag list in the tag management screen is using a one-time memoized snapshot, so it does not refresh after rename/delete/prune actions. Update the TagManage screen to re-query tags after each mutation instead of relying on useMemo(() => getTagsWithCount(), []), and make sure the list state is refreshed when returning from those actions so the UI reflects the latest names and counts.subtrack/src/tui/screens/config.tsx-10-10 (1)
10-10: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRefresh the in-memory config after a save.
configis memoized once, so the screen keeps showing the old value aftersetConfig()succeeds. The change only becomes visible after leaving and re-entering this screen.Also applies to: 24-24, 29-33, 49-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/config.tsx` at line 10, The config screen is holding a stale in-memory snapshot because useMemo(() => loadConfig(), []) only runs once, so updates made by setConfig() never re-render with the latest data. Update the Config screen logic around loadConfig, useMemo, and setConfig so the screen refreshes the config state immediately after a successful save, either by reloading the config into local state or by using a state update mechanism that triggers a re-render.subtrack/src/tui/screens/help.tsx-20-20 (1)
20-20: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThis help line repeats the last shortcut number.
Array.from(...).join(" ")already includes the final value, so appending{SIDEBAR_ITEMS.length}renders the last number twice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/help.tsx` at line 20, The help text in the help screen is duplicating the last shortcut number because the rendered sequence already includes it; update the Text output in the help screen component to stop appending the extra length value after the Array.from(...).join(" ") result. Use the help screen JSX in the TUI help component to locate and adjust the shortcut list rendering so the numbers appear only once.subtrack/src/tui/components/command-bar.tsx-10-10 (1)
10-10: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the delete-screen hint text.
DeleteScreenhandlesy/n, but the command bar tells users to useSpace,Enter, andEsc. On that screen the help text is actively wrong.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/components/command-bar.tsx` at line 10, The delete-screen hint text in the command bar is incorrect for the DeleteScreen flow. Update the delete entry in the command-bar component so it matches the actual DeleteScreen controls handled by the relevant confirm/cancel logic (y/n) instead of Space/Enter/Esc, keeping the rest of the shortcut hints consistent with the command bar labels.subtrack/src/tui/components/sidebar.tsx-51-61 (1)
51-61: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove implemented screens from the placeholder set.
This still marks working screens like
search,add,edit,delete,compare, and others as placeholders, so the sidebar renders much of the new TUI surface dimmed as if it were unavailable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/components/sidebar.tsx` around lines 51 - 61, Update the PLACEHOLDER_SCREENS set in sidebar.tsx so it only contains truly unimplemented screens; remove implemented routes like search, add, edit, delete, compare, and any other working views that are currently being treated as placeholders. Keep the fix centered on PLACEHOLDER_SCREENS and the sidebar rendering logic so the TUI no longer dims available screens as if they were unavailable.subtrack/src/tui/screens/compare.tsx-12-19 (1)
12-19: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRound after aggregating the prorated totals.
Each subscription is rounded before it is added to the per-currency sum. That skews monthly comparisons for quarterly/yearly plans; e.g. two yearly 100-cent subscriptions become 16 cents/month here instead of 17. Accumulate the fractional amount first and round once per currency at the end. Based on learnings, prices are stored as integers in the smallest currency unit.
💡 Suggested fix
const monthlyTotal = useMemo(() => { - const map = new Map<string, number>() + const map = new Map<string, number>() for (const sub of active) { const factor = sub.cycle === "weekly" ? 52/12 : sub.cycle === "bi-weekly" ? 26/12 : sub.cycle === "quarterly" ? 4/12 : sub.cycle === "semi-annual" ? 2/12 : sub.cycle === "yearly" ? 1/12 : 1 - map.set(sub.currency, (map.get(sub.currency) ?? 0) + Math.round(sub.price * factor)) + map.set(sub.currency, (map.get(sub.currency) ?? 0) + sub.price * factor) } - return map + return new Map( + Array.from(map.entries()).map(([currency, total]) => [currency, Math.round(total)]), + ) }, [active])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/compare.tsx` around lines 12 - 19, The monthlyTotal calculation in compare.tsx is rounding each subscription before summing, which skews per-currency totals for prorated plans. Update the useMemo logic so it accumulates the fractional prorated amount for each sub.cycle in the Map first, then apply Math.round once per currency after the loop. Keep the fix localized to monthlyTotal and preserve the existing cycle-to-factor mapping.Source: Learnings
🧹 Nitpick comments (10)
subtrack/src/tui.tsx (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd JSDoc for
handleTui().This is an exported entrypoint, so it should be documented in-module like the other public APIs in this package.
Suggested change
+/** + * Launches the interactive TUI and waits until it exits. + */ export async function handleTui(): Promise<void> {As per coding guidelines, "Document public APIs with JSDoc comments in JavaScript/TypeScript".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui.tsx` at line 4, The exported entrypoint handleTui() is missing in-module JSDoc documentation. Add a concise JSDoc comment directly above handleTui describing its purpose and any important behavior, following the same public-API documentation style used elsewhere in the package.Source: Coding guidelines
subtrack/src/tui/components/status-bar.tsx (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd JSDoc for the exported component.
As per coding guidelines, "Document public APIs with JSDoc comments in JavaScript/TypeScript."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/components/status-bar.tsx` at line 5, The exported StatusBar component is a public API and is missing JSDoc. Add a JSDoc comment immediately above StatusBar to document its purpose and any relevant behavior, following the project guideline for exported components.Source: Coding guidelines
subtrack/src/tui/screens/payment.tsx (1)
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd JSDoc for the exported screen.
As per coding guidelines, "Document public APIs with JSDoc comments in JavaScript/TypeScript."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/payment.tsx` at line 10, The exported PaymentScreen public API is missing JSDoc, so add a brief JSDoc block immediately above PaymentScreen to document what the screen does and keep it consistent with the project’s public API guidelines.Source: Coding guidelines
subtrack/src/tui/screens/subscription-form.tsx (1)
120-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd JSDoc for the exported component.
As per coding guidelines, "Document public APIs with JSDoc comments in JavaScript/TypeScript."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/subscription-form.tsx` at line 120, Add a JSDoc comment for the exported SubscriptionForm component to document this public API per the coding guidelines. Place the comment directly above SubscriptionForm and describe its purpose and props at a high level so the exported function is documented consistently with other public components.Source: Coding guidelines
subtrack/src/tui/screens/analytics.tsx (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd JSDoc for the exported screen.
As per coding guidelines, "Document public APIs with JSDoc comments in JavaScript/TypeScript."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/analytics.tsx` at line 15, The exported AnalyticsScreen public API is missing JSDoc documentation. Add a JSDoc comment immediately above AnalyticsScreen describing its purpose and any relevant behavior so the exported screen is documented per the coding guidelines.Source: Coding guidelines
subtrack/src/tui/screens/forecast.tsx (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd JSDoc for the exported screen.
As per coding guidelines, "Document public APIs with JSDoc comments in JavaScript/TypeScript."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/forecast.tsx` at line 5, Add a JSDoc comment for the exported ForecastScreen component to document this public API per the coding guidelines. Place the comment directly above ForecastScreen so it stays attached to the export and clearly describes the screen’s purpose.Source: Coding guidelines
subtrack/src/tui/screens/tags.tsx (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd JSDoc for this exported screen.
TagsScreenis exported without API documentation. As per coding guidelines, "Document public APIs with JSDoc comments in JavaScript/TypeScript".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/tags.tsx` at line 7, TagsScreen is exported without API documentation; add a JSDoc comment directly above the TagsScreen function to document this public screen API. Keep the documentation concise but clear about what TagsScreen renders/represents, and ensure the comment stays with the exported function so it remains discoverable if the implementation moves.Source: Coding guidelines
subtrack/src/tui/screens/export.tsx (1)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd JSDoc for this exported screen.
ExportScreenis a public API in this module and should be documented. As per coding guidelines, "Document public APIs with JSDoc comments in JavaScript/TypeScript".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/export.tsx` at line 14, Add a JSDoc comment for the exported ExportScreen function so this public screen is documented per the project guidelines. Place the JSDoc directly above ExportScreen and briefly describe that it renders the export TUI screen and any notable behavior or responsibilities of the component.Source: Coding guidelines
subtrack/src/commands.ts (1)
12-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the exported command handler.
handleTuiis a public API in this module and should carry JSDoc. As per coding guidelines, "Document public APIs with JSDoc comments in JavaScript/TypeScript".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/commands.ts` around lines 12 - 15, The exported public API handleTui in commands.ts is missing JSDoc, so add a concise JSDoc comment directly above the handleTui function explaining that it loads and delegates to the TUI handler from tui.tsx. Keep the documentation on the exported command handler itself so the module’s public API is properly documented per the guideline.Source: Coding guidelines
subtrack/src/tui/components/command-bar.tsx (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
import typeforScreen.
Screenis only used in a type position, and the repo guidance asks forimport typein that case. As per coding guidelines,subtrack/**/*.{ts,tsx}should “Useimport typefor type-only imports, e.g.import type { X } from "./foo.ts".”♻️ Suggested fix
-import { type Screen } from "../types.ts" +import type { Screen } from "../types.ts"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/components/command-bar.tsx` around lines 1 - 3, The command-bar module imports Screen as a value import even though it is only used in a type position. Update the import in command-bar.tsx to use import type for Screen alongside the existing imports from ink and app-context, keeping the component logic unchanged.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 95b5ef62-a804-4fc6-b74a-0d41ac5c5a97
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
subtrack/CLAUDE.mdsubtrack/package.jsonsubtrack/src/commands.tssubtrack/src/index.tssubtrack/src/tui.tsxsubtrack/src/tui/app.tsxsubtrack/src/tui/components/command-bar.tsxsubtrack/src/tui/components/sidebar.tsxsubtrack/src/tui/components/status-bar.tsxsubtrack/src/tui/context/app-context.tsxsubtrack/src/tui/screens/add.tsxsubtrack/src/tui/screens/analytics.tsxsubtrack/src/tui/screens/backup.tsxsubtrack/src/tui/screens/bulk.tsxsubtrack/src/tui/screens/compare.tsxsubtrack/src/tui/screens/config.tsxsubtrack/src/tui/screens/delete.tsxsubtrack/src/tui/screens/edit.tsxsubtrack/src/tui/screens/export.tsxsubtrack/src/tui/screens/forecast.tsxsubtrack/src/tui/screens/help.tsxsubtrack/src/tui/screens/import.tsxsubtrack/src/tui/screens/list.tsxsubtrack/src/tui/screens/payment.tsxsubtrack/src/tui/screens/restore.tsxsubtrack/src/tui/screens/search.tsxsubtrack/src/tui/screens/subscription-form.tsxsubtrack/src/tui/screens/summary.tsxsubtrack/src/tui/screens/tag-manage.tsxsubtrack/src/tui/screens/tags.tsxsubtrack/src/tui/screens/trial-add.tsxsubtrack/src/tui/screens/trial-expiring.tsxsubtrack/src/tui/screens/trials.tsxsubtrack/src/tui/screens/upcoming.tsxsubtrack/src/tui/screens/usage.tsxsubtrack/src/tui/types.ts
- Add useMouse hook: enables SGR mouse mode (1006), parses escape sequences for click position - Sidebar items clickable: maps terminal row to sidebar item index for navigation - List items clickable: selects subscription at clicked row in content area - Form screens (add/edit/delete) skip mouse handlers to avoid accidental navigation
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
subtrack/src/tui/screens/forecast.tsx (1)
13-15: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe forecast math rounds away cents before summing.
Math.round(sub.price * f)turns each converted subscription into a whole currency unit before aggregation. With the current major-unit prices, that makes outputs like$14.99 weeklydisplay as$65/monthinstead of$64.96, and the error compounds across subscriptions. Keep the fractional total here and let the formatter round for display, or move the calculation to minor units end-to-end. As per coding guidelines,subtrack/**/*.{ts,tsx}: Represent prices as integers in the smallest unit (for example, JPY without decimals and USD in cents).Also applies to: 22-24
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/forecast.tsx` around lines 13 - 15, The forecast total in forecast should not round each subscription before aggregation, since Math.round(sub.price * f) drops cents and accumulates error. Update the calculation in the forecast screen to keep fractional values until display time, or convert the pricing flow in the relevant forecast logic to use minor units end-to-end, and verify the currency formatter handles final rounding. Use the existing identifiers sub.cycle, sub.price, map, and the forecast rendering in forecast.tsx to locate the affected calculation.Source: Coding guidelines
subtrack/src/tui/screens/payment.tsx (1)
15-29: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRound after aggregating each currency.
These totals round each converted subscription before adding it to the currency bucket. For fractional cycles like yearly→monthly, that can drift by cents once multiple subscriptions share a currency. Accumulate the exact converted amount per currency and round once at the end.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@subtrack/src/tui/screens/payment.tsx` around lines 15 - 29, The monthlyByCurrency and yearlyByCurrency aggregations are rounding each subscription before summing, which can introduce cent-level drift across multiple items in the same currency. Update the payment screen logic in the useMemo blocks to accumulate the exact converted amounts per currency first, then apply rounding once after the per-currency total is computed. Keep the change localized to the monthlyByCurrency and yearlyByCurrency calculations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@subtrack/package.json`:
- Around line 54-55: The lint-staged config for the TypeScript glob is disabling
all checks, which bypasses pre-commit validation. Update the lint-staged entry
for the `*.{ts,tsx}` pattern in package.json to restore the existing TS tasks
and only remove the `typos` step if that was the intent, keeping the other
staged checks intact.
In `@subtrack/src/price.ts`:
- Around line 4-8: The shared price formatter contract in formatPrice currently
assumes major-unit numbers, which conflicts with the repo’s
integer-smallest-unit rule for subtrack ts/tsx code. Update formatPrice to
accept minor-unit integers instead of floats, adjust its doc/signature and any
callers to pass cents/smallest units, and perform the division only inside the
formatter when rendering the display string.
In `@subtrack/src/tui/app.tsx`:
- Around line 120-125: `config` is not included in the global form-screen
guards, so `ConfigScreen` can still be interrupted by app-level shortcuts and
sidebar clicks while its `TextInput` is active. Update both `isFormScreen`
checks in `App` to treat `state.screen === "config"` as a form screen, or
extract the shared predicate into one helper used by both handlers so the
`add/edit/delete/restore/tag-manage/config` list stays consistent.
In `@subtrack/src/tui/hooks/use-mouse.ts`:
- Around line 43-57: The mouse event parser in useMouse is slicing buf.current
with re.lastIndex after the loop, but that value is reset by the final failed
exec on the global regex. Preserve the last successful match offset inside the
while loop in useMouse, then use that saved index when trimming the buffer so
processed events are consumed correctly and old events are not replayed.
In `@subtrack/src/tui/screens/compare.tsx`:
- Around line 19-20: The second pass in the compare screen is still rounding
away cents by mutating the totals in the currency map, so the monthly comparison
remains inaccurate. Update the compare flow in compare.tsx so the aggregation
keeps prices in smallest units end-to-end (preferred per subtrack guidelines)
or, if the values are already in major units, remove the final Math.round-style
normalization and leave display rounding to formatPrice. Keep the fix centered
around the monthly totals map iteration in the compare screen.
In `@subtrack/src/tui/screens/upcoming.tsx`:
- Around line 52-55: The date comparison in upcoming.tsx is using a midnight
Date for d against a from value that still includes the current time, which
causes bills due today to roll forward after midnight. Update the logic around
the d/from comparison so it compares day-level dates only, or normalizes from to
the same start-of-day before checking d < from, and keep the fix within the date
calculation block that builds d and handles the next month fallback.
---
Outside diff comments:
In `@subtrack/src/tui/screens/forecast.tsx`:
- Around line 13-15: The forecast total in forecast should not round each
subscription before aggregation, since Math.round(sub.price * f) drops cents and
accumulates error. Update the calculation in the forecast screen to keep
fractional values until display time, or convert the pricing flow in the
relevant forecast logic to use minor units end-to-end, and verify the currency
formatter handles final rounding. Use the existing identifiers sub.cycle,
sub.price, map, and the forecast rendering in forecast.tsx to locate the
affected calculation.
In `@subtrack/src/tui/screens/payment.tsx`:
- Around line 15-29: The monthlyByCurrency and yearlyByCurrency aggregations are
rounding each subscription before summing, which can introduce cent-level drift
across multiple items in the same currency. Update the payment screen logic in
the useMemo blocks to accumulate the exact converted amounts per currency first,
then apply rounding once after the per-currency total is computed. Keep the
change localized to the monthlyByCurrency and yearlyByCurrency calculations.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c282440-6561-4cb2-b955-2b69616f736c
📒 Files selected for processing (23)
subtrack/package.jsonsubtrack/src/display.tssubtrack/src/price.tssubtrack/src/tui/app.tsxsubtrack/src/tui/components/command-bar.tsxsubtrack/src/tui/components/sidebar.tsxsubtrack/src/tui/hooks/use-mouse.tssubtrack/src/tui/screens/analytics.tsxsubtrack/src/tui/screens/backup.tsxsubtrack/src/tui/screens/compare.tsxsubtrack/src/tui/screens/config.tsxsubtrack/src/tui/screens/forecast.tsxsubtrack/src/tui/screens/help.tsxsubtrack/src/tui/screens/import.tsxsubtrack/src/tui/screens/list.tsxsubtrack/src/tui/screens/payment.tsxsubtrack/src/tui/screens/restore.tsxsubtrack/src/tui/screens/subscription-form.tsxsubtrack/src/tui/screens/summary.tsxsubtrack/src/tui/screens/tag-manage.tsxsubtrack/src/tui/screens/trial-add.tsxsubtrack/src/tui/screens/trials.tsxsubtrack/src/tui/screens/upcoming.tsx
🚧 Files skipped from review as they are similar to previous changes (11)
- subtrack/src/tui/screens/help.tsx
- subtrack/src/tui/screens/backup.tsx
- subtrack/src/tui/components/command-bar.tsx
- subtrack/src/tui/screens/subscription-form.tsx
- subtrack/src/tui/screens/trial-add.tsx
- subtrack/src/tui/screens/trials.tsx
- subtrack/src/tui/screens/tag-manage.tsx
- subtrack/src/tui/components/sidebar.tsx
- subtrack/src/tui/screens/list.tsx
- subtrack/src/tui/screens/restore.tsx
- subtrack/src/tui/screens/import.tsx
| "lint-staged": { | ||
| "*.{ts,tsx}": [ | ||
| "typos --", | ||
| "tsc --noEmit" | ||
| ], | ||
| "*.{ts,tsx,json,md,yaml,yml}": [ | ||
| "typos --" | ||
| ] | ||
| "*.{ts,tsx}": [] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect the relevant section with line numbers.
sed -n '1,160p' subtrack/package.json | cat -nRepository: nazozokc/subtrack
Length of output: 2780
🌐 Web query:
lint-staged configuration empty array meaning for a glob
💡 Result:
In lint-staged, a glob pattern is expected to be a string that identifies a subset of staged files to process [1][2]. There is no native feature or documented behavior that interprets an empty array ([]) as a valid glob pattern for file matching; typically, configuration keys are expected to be strings representing valid glob patterns [1][3]. If you provide an empty array as a key in your configuration object, it will not match any files. Consequently, the commands associated with that key will not be executed, effectively acting as a no-op (no operation) for that specific configuration entry. If you are attempting to disable a lint-staged rule or prevent tasks from running, the correct approach is to remove the key-value pair from your configuration object entirely [1][2]. If you need to conditionally control which tasks run based on dynamic logic, you should use a JavaScript configuration file (e.g., lint-staged.config.js) and export a function instead of an object [1][2]. This allows you to programmatically return an empty object or skip command registration when certain conditions are met [1][3].
Citations:
- 1: https://www.npmjs.com/package/lint-staged
- 2: https://github.com/lint-staged/lint-staged/blob/main/README.md
- 3: https://github.com/lint-staged/lint-staged?tab=readme-ov-file
Restore the staged TS tasks. *.{ts,tsx}: [] turns off all lint-staged checks for TypeScript files, so these changes bypass pre-commit validation entirely. Keep the remaining tasks on this glob if the only goal is to skip typos.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@subtrack/package.json` around lines 54 - 55, The lint-staged config for the
TypeScript glob is disabling all checks, which bypasses pre-commit validation.
Update the lint-staged entry for the `*.{ts,tsx}` pattern in package.json to
restore the existing TS tasks and only remove the `typos` step if that was the
intent, keeping the other staged checks intact.
| * Currently prices are stored as major units (e.g. 14.99 USD → "$14.99"). | ||
| * This function formats consistently across CLI and TUI surfaces. | ||
| * When migrating to smallest-unit storage (cents), add a `fromCents` parameter. | ||
| */ | ||
| export function formatPrice(price: number, currency: string): string { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not bake major-unit prices into the shared formatter contract.
This helper is now the shared boundary for price rendering, but its doc/signature explicitly standardize on major-unit numbers. That conflicts with the repo rule for subtrack/**/*.{ts,tsx} and locks the new TUI screens into float math and rounding drift. Please switch this API to accept minor-unit integers and divide only when formatting. As per coding guidelines, subtrack/**/*.{ts,tsx}: Represent prices as integers in the smallest unit (for example, JPY without decimals and USD in cents).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@subtrack/src/price.ts` around lines 4 - 8, The shared price formatter
contract in formatPrice currently assumes major-unit numbers, which conflicts
with the repo’s integer-smallest-unit rule for subtrack ts/tsx code. Update
formatPrice to accept minor-unit integers instead of floats, adjust its
doc/signature and any callers to pass cents/smallest units, and perform the
division only inside the formatter when rendering the display string.
Source: Coding guidelines
| const isFormScreen = | ||
| state.screen === "add" || | ||
| state.screen === "edit" || | ||
| state.screen === "delete" || | ||
| state.screen === "restore" || | ||
| state.screen === "tag-manage" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Treat config as a form screen in both global handlers.
ConfigScreen mounts TextInput and its own useInput, but config is missing from both isFormScreen checks. While editing a value, global q/j/k/: bindings and sidebar clicks can still fire and pull the user out of the form. Add state.screen === "config" to both guards, or centralize the predicate so the lists cannot drift again.
Also applies to: 453-458
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@subtrack/src/tui/app.tsx` around lines 120 - 125, `config` is not included in
the global form-screen guards, so `ConfigScreen` can still be interrupted by
app-level shortcuts and sidebar clicks while its `TextInput` is active. Update
both `isFormScreen` checks in `App` to treat `state.screen === "config"` as a
form screen, or extract the shared predicate into one helper used by both
handlers so the `add/edit/delete/restore/tag-manage/config` list stays
consistent.
| while ((match = re.exec(buf.current)) !== null) { | ||
| last = match | ||
| } | ||
|
|
||
| if (last) { | ||
| const [, rawBtn, rawX, rawY, kind] = last | ||
| const btn = parseInt(rawBtn, 10) | ||
| setClick({ | ||
| x: parseInt(rawX, 10), | ||
| y: parseInt(rawY, 10), | ||
| button: btn & 3, // 0=left, 1=middle, 2=right | ||
| pressed: kind === "M", // M=press, m=release | ||
| }) | ||
| // Consume processed data up to the end of the last match | ||
| buf.current = buf.current.slice(re.lastIndex) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target file and inspect the relevant region with line numbers.
git ls-files 'subtrack/src/tui/hooks/use-mouse.ts'
wc -l subtrack/src/tui/hooks/use-mouse.ts
sed -n '1,140p' subtrack/src/tui/hooks/use-mouse.tsRepository: nazozokc/subtrack
Length of output: 2260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const buf = "\x1b[<0;12;34Mxxx\x1b[<1;56;78m"
const re = /\x1b\[<(\d+);(\d+);(\d+)([Mm])/g
let match = null
let last = null
while ((match = re.exec(buf)) !== null) {
last = match
console.log("match", match[0], "lastIndex", re.lastIndex)
}
console.log("after loop lastIndex", re.lastIndex)
console.log("last match end", last?.index + last?.[0].length)
JSRepository: nazozokc/subtrack
Length of output: 258
Preserve the consumed offset before slicing the buffer.
re.lastIndex resets to 0 after the final failed exec() on a global regex, so buf.current.slice(re.lastIndex) keeps the entire buffer here. That replays old mouse events and lets the buffer grow without bound. Store the last successful re.lastIndex and slice from that instead.
Possible fix
const re = /\x1b\[<(\d+);(\d+);(\d+)([Mm])/g
let match: RegExpExecArray | null = null
let last: RegExpExecArray | null = null
+ let consumed = 0
while ((match = re.exec(buf.current)) !== null) {
last = match
+ consumed = re.lastIndex
}
@@
- buf.current = buf.current.slice(re.lastIndex)
+ buf.current = buf.current.slice(consumed)
}📝 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.
| while ((match = re.exec(buf.current)) !== null) { | |
| last = match | |
| } | |
| if (last) { | |
| const [, rawBtn, rawX, rawY, kind] = last | |
| const btn = parseInt(rawBtn, 10) | |
| setClick({ | |
| x: parseInt(rawX, 10), | |
| y: parseInt(rawY, 10), | |
| button: btn & 3, // 0=left, 1=middle, 2=right | |
| pressed: kind === "M", // M=press, m=release | |
| }) | |
| // Consume processed data up to the end of the last match | |
| buf.current = buf.current.slice(re.lastIndex) | |
| const re = /\x1b\[<(\d+);(\d+);(\d+)([Mm])/g | |
| let match: RegExpExecArray | null = null | |
| let last: RegExpExecArray | null = null | |
| let consumed = 0 | |
| while ((match = re.exec(buf.current)) !== null) { | |
| last = match | |
| consumed = re.lastIndex | |
| } | |
| if (last) { | |
| const [, rawBtn, rawX, rawY, kind] = last | |
| const btn = parseInt(rawBtn, 10) | |
| setClick({ | |
| x: parseInt(rawX, 10), | |
| y: parseInt(rawY, 10), | |
| button: btn & 3, // 0=left, 1=middle, 2=right | |
| pressed: kind === "M", // M=press, m=release | |
| }) | |
| // Consume processed data up to the end of the last match | |
| buf.current = buf.current.slice(consumed) |
🧰 Tools
🪛 OpenGrep (1.23.0)
[ERROR] 43-43: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@subtrack/src/tui/hooks/use-mouse.ts` around lines 43 - 57, The mouse event
parser in useMouse is slicing buf.current with re.lastIndex after the loop, but
that value is reset by the final failed exec on the global regex. Preserve the
last successful match offset inside the while loop in useMouse, then use that
saved index when trimming the buffer so processed events are consumed correctly
and old events are not replayed.
| for (const [currency, total] of map) { | ||
| map.set(currency, Math.round(total)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The monthly comparison still drops cents in the final pass.
This second pass rounds each currency total to a whole unit, so the screen still shows off-by-cents monthly totals. Leave the aggregated value unrounded here and let formatPrice handle display rounding, or switch the whole flow to minor units. As per coding guidelines, subtrack/**/*.{ts,tsx}: Represent prices as integers in the smallest unit (for example, JPY without decimals and USD in cents).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@subtrack/src/tui/screens/compare.tsx` around lines 19 - 20, The second pass
in the compare screen is still rounding away cents by mutating the totals in the
currency map, so the monthly comparison remains inaccurate. Update the compare
flow in compare.tsx so the aggregation keeps prices in smallest units end-to-end
(preferred per subtrack guidelines) or, if the values are already in major
units, remove the final Math.round-style normalization and leave display
rounding to formatPrice. Keep the fix centered around the monthly totals map
iteration in the compare screen.
Source: Coding guidelines
| let d = new Date(year, month, clampedDay) | ||
| if (d < from) { | ||
| const nextLastDay = new Date(year, month + 2, 0).getDate() | ||
| d = new Date(year, month + 1, Math.min(day, nextLastDay)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Bills due today get skipped after midnight.
d is created at 00:00, but from keeps the current time. On the billing day itself, d < from becomes true after midnight and this incorrectly rolls the next bill into the following month.
💡 Suggested fix
function computeNextBill(day: number, from: Date, until: Date): Date | null {
- const year = from.getFullYear()
- const month = from.getMonth()
+ const fromDate = new Date(from.getFullYear(), from.getMonth(), from.getDate())
+ const year = fromDate.getFullYear()
+ const month = fromDate.getMonth()
const lastDay = new Date(year, month + 1, 0).getDate()
const clampedDay = Math.min(day, lastDay)
let d = new Date(year, month, clampedDay)
- if (d < from) {
+ if (d < fromDate) {
const nextLastDay = new Date(year, month + 2, 0).getDate()
d = new Date(year, month + 1, Math.min(day, nextLastDay))
}📝 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.
| let d = new Date(year, month, clampedDay) | |
| if (d < from) { | |
| const nextLastDay = new Date(year, month + 2, 0).getDate() | |
| d = new Date(year, month + 1, Math.min(day, nextLastDay)) | |
| const fromDate = new Date(from.getFullYear(), from.getMonth(), from.getDate()) | |
| const year = fromDate.getFullYear() | |
| const month = fromDate.getMonth() | |
| let d = new Date(year, month, clampedDay) | |
| if (d < fromDate) { | |
| const nextLastDay = new Date(year, month + 2, 0).getDate() | |
| d = new Date(year, month + 1, Math.min(day, nextLastDay)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@subtrack/src/tui/screens/upcoming.tsx` around lines 52 - 55, The date
comparison in upcoming.tsx is using a midnight Date for d against a from value
that still includes the current time, which causes bills due today to roll
forward after midnight. Update the logic around the d/from comparison so it
compares day-level dates only, or normalizes from to the same start-of-day
before checking d < from, and keep the fix within the date calculation block
that builds d and handles the next month fallback.
nazozokc
changed the title
1 participant
Summary
Add interactive TUI mode (
subtrack tui) with left sidebar navigation, vim-like keybindings, mouse click support, and screens for all CLI features.Changes
TUI Infrastructure
Screens
Quality & Polish
Technical