Conversation
- Add .npmrc with engine-strict and strict-peer-dependencies - Add package metadata (description, keywords, homepage, bugs) - Set engines.node >=22 and sideEffects: false - Enable OSV vulnerability alerts in Renovate with npm:unpublishSafe - Add pnpm audit gate to release workflow - Replace allowBuilds with onlyBuiltDependencies in workspace config
|
Warning Review limit reached
More reviews will be available in 39 minutes and 22 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 refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughThe PR introduces LLM API usage tracking (pricing lookup, ChangesSource code refactoring and LLM usage feature
Package hardening and CI security
Sequence Diagram(s)sequenceDiagram
actor User
participant index.ts
participant usage.ts
participant pricing.ts
participant db.ts
rect rgba(70, 130, 180, 0.5)
Note over User,db.ts: usage add
User->>index.ts: subtrack usage add [flags]
index.ts->>usage.ts: handleUsageAdd(flags)
usage.ts->>pricing.ts: ensurePricingCache()
pricing.ts-->>usage.ts: PricingCache | null
usage.ts->>pricing.ts: matchModel(cache, provider, model)
pricing.ts-->>usage.ts: ModelPricingEntry | null
usage.ts->>pricing.ts: calculateCostCents(pricing, inputTokens, outputTokens)
pricing.ts-->>usage.ts: cost in cents
usage.ts->>db.ts: addLlmUsage(result)
db.ts-->>usage.ts: void
usage.ts-->>User: success log
end
rect rgba(60, 179, 113, 0.5)
Note over User,db.ts: payment --api
User->>index.ts: subtrack payment --api
index.ts->>usage.ts: handlePayment(period, {api: true})
usage.ts->>db.ts: getLlmUsageTotal(from, to)
db.ts-->>usage.ts: total cost
usage.ts->>db.ts: getLlmUsageTotalByProvider(from, to)
db.ts-->>usage.ts: [{provider, total}]
usage.ts-->>User: payment table + API usage breakdown
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 12
🧹 Nitpick comments (5)
subtrack/src/pricing.ts (1)
60-69: 💤 Low valueConsider adding a timeout to the fetch request.
The
fetchcall to GitHub has no timeout, which could cause the CLI to hang indefinitely if the network is slow or unresponsive. For a CLI tool this is minor since users can Ctrl+C, but addingAbortSignal.timeout()would improve UX.💡 Optional: Add fetch timeout
// Fetch from GitHub try { - const res = await fetch(GITHUB_JSON_URL) + const res = await fetch(GITHUB_JSON_URL, { signal: AbortSignal.timeout(15_000) }) if (!res.ok) throw new Error(`GitHub responded with ${res.status}`)🤖 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/pricing.ts` around lines 60 - 69, The fetch request to GITHUB_JSON_URL lacks a timeout configuration, which could cause the CLI to hang indefinitely on slow or unresponsive networks. Add an AbortSignal with a timeout to the fetch call by passing a signal option to the fetch function with AbortSignal.timeout() specifying an appropriate timeout duration in milliseconds (typically 5000-10000ms for network requests). This will automatically abort the request if it exceeds the specified time limit.subtrack/src/export.ts (1)
15-39: ⚡ Quick winDocument exported serializer APIs with JSDoc.
exportCsv,exportJson, andexportMdare public module APIs but currently undocumented.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/export.ts` around lines 15 - 39, Add JSDoc comments to the three public export functions: exportCsv, exportJson, and exportMd. Each function needs a JSDoc block that describes what the function does, documents the subs parameter (SharedArgs array), and specifies the return type (string). The JSDoc comments should clearly explain the purpose of each export function and what format it produces.Source: Coding guidelines
subtrack/src/types.ts (2)
49-55: ⚡ Quick winUnify
GetLlmUsageOptionsto one exported type.
GetLlmUsageOptionsis defined here and also insubtrack/src/db.ts(context snippet). Keeping both definitions risks silent contract drift between callers and DB filtering logic.♻️ Suggested consolidation
-// subtrack/src/db.ts -export type GetLlmUsageOptions = { - provider?: string - from?: string - to?: string - limit?: number - offset?: number -} +// subtrack/src/db.ts +import type { GetLlmUsageOptions } 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/types.ts` around lines 49 - 55, The GetLlmUsageOptions type is currently defined in both subtrack/src/types.ts and subtrack/src/db.ts, creating a risk of divergence between the two definitions. Remove the duplicate GetLlmUsageOptions type definition from subtrack/src/db.ts and instead import it from subtrack/src/types.ts at the top of the db.ts file. This ensures there is a single source of truth for the type contract used by the DB filtering logic.
25-35: ⚡ Quick winAdd JSDoc to exported public API types in this module.
Most exported public type aliases here are undocumented, which weakens API discoverability after the type consolidation.
As per coding guidelines, "Document public APIs with JSDoc comments in JavaScript/TypeScript".
Also applies to: 36-55, 57-72
🤖 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/types.ts` around lines 25 - 35, Add JSDoc documentation comments to all exported public type aliases in the module, including SharedArgs, AddSharedArgs, and all other exported types mentioned in the comment (those at lines 36-55 and 57-72). For each type, provide a clear description of what the type represents and its purpose in the API. Place the JSDoc comment block immediately above each type export to improve API discoverability and developer understanding.Source: Coding guidelines
subtrack/src/import-csv.ts (1)
8-43: ⚡ Quick winAdd JSDoc for exported import APIs.
parseCsvLineandhandleImportare exported public APIs 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/import-csv.ts` around lines 8 - 43, The exported functions parseCsvLine and handleImport lack JSDoc documentation required for public APIs. Add JSDoc comments above each function that describe their purpose, document all parameters with their types and descriptions, and document the return type. For parseCsvLine, document that it takes a CSV line string and returns an array of parsed field strings. For handleImport, document the file parameter and options parameter (including the dryRun option), and specify its return type as a Promise.Source: Coding guidelines
🤖 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 @.github/workflows/release.yml:
- Around line 52-53: The audit step runs at the workspace root checking all
packages, but the publish is scoped only to ./subtrack, causing unrelated
vulnerabilities to block the release. Scope the pnpm audit command to only the
./subtrack package by adding the appropriate filter flag to the run command that
executes "pnpm audit --audit-level=high" so it matches the scope of the publish
step and only validates dependencies relevant to the package being released.
In `@subtrack/src/db.test.ts`:
- Around line 727-737: The test for getLlmUsage only validates descending date
ordering but does not test the tie-breaking behavior when multiple entries share
the same date. To address this, add additional test entries with identical dates
to the test function getLlmUsage returns entries ordered by date desc and add
assertions that verify these same-date entries are ordered by id in descending
order. This ensures the id DESC tie-break logic in the getLlmUsage query is
properly tested.
In `@subtrack/src/display.test.ts`:
- Around line 625-633: Replace the dynamic date generation using new
Date().toISOString().split("T")[0] with a fixed hardcoded date string (for
example, "2024-01-15") in the test "shows API usage in USD without --currency"
where addLlmUsage is called with the date parameter. This ensures deterministic
test behavior independent of timezone or when the test runs. Apply the same fix
to all other occurrences of this pattern in the test file, particularly in the
test cases referenced at lines 645-653.
- Around line 5-6: The import statement for spreadSubscription from the local
module does not include the .ts file extension, which is inconsistent with the
coding guidelines that require .ts extensions for local imports. Update the
import statement on line 5 to add the .ts extension to the ./display module
path, making it consistent with the pattern used in the types.ts import on line
6.
In `@subtrack/src/export.ts`:
- Around line 8-13: The escapeCsv function does not protect against formula
injection attacks where user input starting with =, +, -, or @ characters can
execute formulas when the CSV is opened in spreadsheet applications. Modify the
escapeCsv function to check if the value starts with any of these dangerous
characters and, if detected, prepend a single quote to neutralize the formula
execution. Apply this check before or after the existing quote/comma/newline
escaping logic to ensure all potentially malicious inputs are properly
neutralized.
In `@subtrack/src/fx.ts`:
- Around line 6-11: The fetchFxRates function has a fetch call that lacks a
timeout mechanism, allowing it to hang indefinitely if the upstream API stalls.
Implement a timeout for the fetch request in fetchFxRates using AbortController
by creating an abort controller, setting a timeout that triggers the abort (with
an appropriate duration like 5-10 seconds), passing the abort signal to the
fetch call, and handling the resulting AbortError that occurs if the timeout
expires before the request completes.
In `@subtrack/src/import-csv.ts`:
- Around line 54-57: The issue is that the code splits the content by newline in
the lines variable before parsing it as CSV, which breaks valid quoted multiline
CSV fields. Instead of performing the split("\n").map((l) =>
l.trim()).filter(Boolean) operation on the clean content, use a proper CSV
parsing library that correctly handles quoted fields containing newlines. Pass
the clean variable to a CSV parser that respects quote escaping rules, so fields
like "Line1\nLine2" remain intact as single fields during import and maintain
round-trip consistency with the exportCsv function.
- Around line 78-85: The condition checking the number of fields in the CSV
import function is too permissive. Currently it only rejects rows with fewer
than 5 fields using `fields.length < 5`, but this allows rows with extra columns
to pass through and get silently truncated during destructuring. Change the
condition to use `fields.length !== 5` instead to ensure rows are rejected
unless they have exactly 5 fields, which will prevent incorrect values from
being imported without explicit error handling.
In `@subtrack/src/payment.ts`:
- Around line 237-246: The monthlyByTag aggregation mixes currency values
without conversion, causing incorrect totals when a single tag contains
subscriptions in different currencies. Refactor the monthlyByTag data structure
to track totals separately by currency for each tag, changing from a flat
monthly amount to a nested structure that groups monthly totals and counts by
currency. Then update the display loop that iterates over sorted entries to
handle the per-currency breakdown, displaying each currency separately under its
tag with the appropriate currency symbol instead of assuming all values are in
USD.
In `@subtrack/src/pricing.test.ts`:
- Around line 126-138: In the test "ensurePricingCache returns null when fetch
fails and no cache", add an actual invocation of the ensurePricingCache function
from the imported pricing module and include assertions to verify it returns
null when fetch fails and no cache exists. Currently the test mocks
globalThis.fetch but never calls the ensurePricingCache function or asserts its
return value, rendering the test unable to validate the function's actual
behavior.
In `@subtrack/src/pricing.ts`:
- Around line 144-154: The calculateCostCents function is applying both
outputCost and reasoningCost to the same outputTokens value, causing
double-charging for output tokens. To fix this, either add a separate
reasoningTokens parameter to the function signature and use it for calculating
reasoningCost instead of outputTokens, or modify the reasoningCost calculation
to only apply when output_cost_per_reasoning_token is defined and make it
mutually exclusive with the standard output_cost_per_token calculation.
Additionally, ensure the corresponding test is updated to validate the corrected
behavior instead of the current double-charging scenario.
In `@subtrack/src/usage.ts`:
- Around line 88-97: The validation function validateDate returns true for empty
strings because !v.trim() evaluates to true when the trimmed value is empty, but
then the empty string gets assigned to the date variable instead of defaulting
to today. After the validateDate check in the conditional block where flags.date
is validated, add an additional check to ensure the trimmed date value is not
empty; if it is empty after trimming, set date to today instead of storing the
empty string. Alternatively, modify the conditional logic to check that
flags.date is not only defined but also has a non-empty trimmed value before
proceeding with validation.
---
Nitpick comments:
In `@subtrack/src/export.ts`:
- Around line 15-39: Add JSDoc comments to the three public export functions:
exportCsv, exportJson, and exportMd. Each function needs a JSDoc block that
describes what the function does, documents the subs parameter (SharedArgs
array), and specifies the return type (string). The JSDoc comments should
clearly explain the purpose of each export function and what format it produces.
In `@subtrack/src/import-csv.ts`:
- Around line 8-43: The exported functions parseCsvLine and handleImport lack
JSDoc documentation required for public APIs. Add JSDoc comments above each
function that describe their purpose, document all parameters with their types
and descriptions, and document the return type. For parseCsvLine, document that
it takes a CSV line string and returns an array of parsed field strings. For
handleImport, document the file parameter and options parameter (including the
dryRun option), and specify its return type as a Promise.
In `@subtrack/src/pricing.ts`:
- Around line 60-69: The fetch request to GITHUB_JSON_URL lacks a timeout
configuration, which could cause the CLI to hang indefinitely on slow or
unresponsive networks. Add an AbortSignal with a timeout to the fetch call by
passing a signal option to the fetch function with AbortSignal.timeout()
specifying an appropriate timeout duration in milliseconds (typically
5000-10000ms for network requests). This will automatically abort the request if
it exceeds the specified time limit.
In `@subtrack/src/types.ts`:
- Around line 49-55: The GetLlmUsageOptions type is currently defined in both
subtrack/src/types.ts and subtrack/src/db.ts, creating a risk of divergence
between the two definitions. Remove the duplicate GetLlmUsageOptions type
definition from subtrack/src/db.ts and instead import it from
subtrack/src/types.ts at the top of the db.ts file. This ensures there is a
single source of truth for the type contract used by the DB filtering logic.
- Around line 25-35: Add JSDoc documentation comments to all exported public
type aliases in the module, including SharedArgs, AddSharedArgs, and all other
exported types mentioned in the comment (those at lines 36-55 and 57-72). For
each type, provide a clear description of what the type represents and its
purpose in the API. Place the JSDoc comment block immediately above each type
export to improve API discoverability and developer understanding.
🪄 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: 6f6a5fd9-993e-441c-846c-5d3667ab36ce
📒 Files selected for processing (22)
.github/renovate.json.github/workflows/release.yml.npmrcpackage.jsonpnpm-workspace.yamlsubtrack/package.jsonsubtrack/src/commands.test.tssubtrack/src/commands.tssubtrack/src/db.test.tssubtrack/src/db.tssubtrack/src/display.test.tssubtrack/src/display.tssubtrack/src/export.tssubtrack/src/fx.tssubtrack/src/import-csv.tssubtrack/src/index.tssubtrack/src/payment.tssubtrack/src/pricing.test.tssubtrack/src/pricing.tssubtrack/src/prompts.tssubtrack/src/types.tssubtrack/src/usage.ts
| - name: Audit dependencies | ||
| run: pnpm audit --audit-level=high |
There was a problem hiding this comment.
Scope the audit step to the package being published (Line 52).
The audit currently runs at workspace root, while publish is scoped to ./subtrack. This can block a valid subtrack release due to vulnerabilities in unrelated workspaces.
Suggested fix
- name: Audit dependencies
- run: pnpm audit --audit-level=high
+ working-directory: ./subtrack
+ run: pnpm audit --audit-level=high📝 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.
| - name: Audit dependencies | |
| run: pnpm audit --audit-level=high | |
| - name: Audit dependencies | |
| working-directory: ./subtrack | |
| run: pnpm audit --audit-level=high |
🤖 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 @.github/workflows/release.yml around lines 52 - 53, The audit step runs at
the workspace root checking all packages, but the publish is scoped only to
./subtrack, causing unrelated vulnerabilities to block the release. Scope the
pnpm audit command to only the ./subtrack package by adding the appropriate
filter flag to the run command that executes "pnpm audit --audit-level=high" so
it matches the scope of the publish step and only validates dependencies
relevant to the package being released.
| test("getLlmUsage returns entries ordered by date desc", async () => { | ||
| const db = await import("./db.ts") | ||
| db.addLlmUsage({ provider: "openai", model: "a", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-01", description: null }) | ||
| db.addLlmUsage({ provider: "openai", model: "b", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-15", description: null }) | ||
| db.addLlmUsage({ provider: "openai", model: "c", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-10", description: null }) | ||
|
|
||
| const entries = db.getLlmUsage() | ||
| expect(entries[0].model).toBe("b") // latest first | ||
| expect(entries[1].model).toBe("c") | ||
| expect(entries[2].model).toBe("a") | ||
| }) |
There was a problem hiding this comment.
Add a same-date tie-break assertion for getLlmUsage ordering.
This test checks date-desc ordering, but not the equal-date id DESC tie-break used by the query. That leaves a deterministic ordering path untested.
As per coding guidelines: "Write unit tests for all functions and critical code paths."
💡 Suggested test addition
test("getLlmUsage returns entries ordered by date desc", async () => {
const db = await import("./db.ts")
db.addLlmUsage({ provider: "openai", model: "a", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-01", description: null })
db.addLlmUsage({ provider: "openai", model: "b", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-15", description: null })
db.addLlmUsage({ provider: "openai", model: "c", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-10", description: null })
const entries = db.getLlmUsage()
expect(entries[0].model).toBe("b") // latest first
expect(entries[1].model).toBe("c")
expect(entries[2].model).toBe("a")
})
+
+test("getLlmUsage uses id desc as tie-breaker for same date", async () => {
+ const db = await import("./db.ts")
+ db.addLlmUsage({ provider: "openai", model: "first", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-15", description: null })
+ db.addLlmUsage({ provider: "openai", model: "second", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-15", description: null })
+
+ const entries = db.getLlmUsage()
+ expect(entries[0].model).toBe("second")
+ expect(entries[1].model).toBe("first")
+})📝 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.
| test("getLlmUsage returns entries ordered by date desc", async () => { | |
| const db = await import("./db.ts") | |
| db.addLlmUsage({ provider: "openai", model: "a", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-01", description: null }) | |
| db.addLlmUsage({ provider: "openai", model: "b", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-15", description: null }) | |
| db.addLlmUsage({ provider: "openai", model: "c", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-10", description: null }) | |
| const entries = db.getLlmUsage() | |
| expect(entries[0].model).toBe("b") // latest first | |
| expect(entries[1].model).toBe("c") | |
| expect(entries[2].model).toBe("a") | |
| }) | |
| test("getLlmUsage returns entries ordered by date desc", async () => { | |
| const db = await import("./db.ts") | |
| db.addLlmUsage({ provider: "openai", model: "a", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-01", description: null }) | |
| db.addLlmUsage({ provider: "openai", model: "b", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-15", description: null }) | |
| db.addLlmUsage({ provider: "openai", model: "c", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-10", description: null }) | |
| const entries = db.getLlmUsage() | |
| expect(entries[0].model).toBe("b") // latest first | |
| expect(entries[1].model).toBe("c") | |
| expect(entries[2].model).toBe("a") | |
| }) | |
| test("getLlmUsage uses id desc as tie-breaker for same date", async () => { | |
| const db = await import("./db.ts") | |
| db.addLlmUsage({ provider: "openai", model: "first", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-15", description: null }) | |
| db.addLlmUsage({ provider: "openai", model: "second", input_tokens: 1, output_tokens: 1, cost: 0.01, date: "2026-06-15", description: null }) | |
| const entries = db.getLlmUsage() | |
| expect(entries[0].model).toBe("second") | |
| expect(entries[1].model).toBe("first") | |
| }) |
🤖 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/db.test.ts` around lines 727 - 737, The test for getLlmUsage
only validates descending date ordering but does not test the tie-breaking
behavior when multiple entries share the same date. To address this, add
additional test entries with identical dates to the test function getLlmUsage
returns entries ordered by date desc and add assertions that verify these
same-date entries are ordered by id in descending order. This ensures the id
DESC tie-break logic in the getLlmUsage query is properly tested.
Source: Coding guidelines
| import { spreadSubscription } from "./display" | ||
| import type { SharedArgs } from "./db" | ||
| import type { SharedArgs } from "./types.ts" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'import\s+.*from\s+"\.{1,2}/[^"]+(?<!\.ts)"' subtrack/src/display.test.tsRepository: nazozokc/subtrack
Length of output: 110
🏁 Script executed:
#!/bin/bash
# Check the exact content of the test file at the specified lines
sed -n '1,15p' subtrack/src/display.test.tsRepository: nazozokc/subtrack
Length of output: 550
🏁 Script executed:
#!/bin/bash
# Verify that the display.ts module exists
ls -la subtrack/src/display.ts 2>/dev/null || echo "display.ts not found"Repository: nazozokc/subtrack
Length of output: 134
🏁 Script executed:
#!/bin/bash
# Check if there are other similar imports in this file
rg -n 'import.*from\s+"[^"]*"' subtrack/src/display.test.tsRepository: nazozokc/subtrack
Length of output: 361
Use .ts extension for this local import.
Line 5 imports from local module ./display without the .ts extension, inconsistent with the coding guidelines requiring .ts extension for local imports in TypeScript files and conflicting with the correct import pattern shown on line 6.
✅ Suggested fix
-import { spreadSubscription } from "./display"
+import { spreadSubscription } from "./display.ts"📝 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.
| import { spreadSubscription } from "./display" | |
| import type { SharedArgs } from "./db" | |
| import type { SharedArgs } from "./types.ts" | |
| import { spreadSubscription } from "./display.ts" | |
| import type { SharedArgs } 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/display.test.ts` around lines 5 - 6, The import statement for
spreadSubscription from the local module does not include the .ts file
extension, which is inconsistent with the coding guidelines that require .ts
extensions for local imports. Update the import statement on line 5 to add the
.ts extension to the ./display module path, making it consistent with the
pattern used in the types.ts import on line 6.
Source: Coding guidelines
| test("shows API usage in USD without --currency", async () => { | ||
| const db = await import("./db.ts") | ||
| db.addLlmUsage({ | ||
| provider: "openai", | ||
| model: "gpt-4o", | ||
| input_tokens: 1000, | ||
| output_tokens: 500, | ||
| cost: 50, // 50 cents = $0.50 | ||
| date: new Date().toISOString().split("T")[0], |
There was a problem hiding this comment.
Avoid real-time dates in these tests to prevent flakiness.
Using new Date().toISOString().split("T")[0] makes assertions sensitive to timezone/month-boundary behavior. Prefer fixed dates (or mocked time) for deterministic test outcomes.
Also applies to: 645-653
🤖 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/display.test.ts` around lines 625 - 633, Replace the dynamic
date generation using new Date().toISOString().split("T")[0] with a fixed
hardcoded date string (for example, "2024-01-15") in the test "shows API usage
in USD without --currency" where addLlmUsage is called with the date parameter.
This ensures deterministic test behavior independent of timezone or when the
test runs. Apply the same fix to all other occurrences of this pattern in the
test file, particularly in the test cases referenced at lines 645-653.
| function escapeCsv(value: string): string { | ||
| if (value.includes('"') || value.includes(",") || value.includes("\n")) { | ||
| return `"${value.replace(/"/g, '""')}"` | ||
| } | ||
| return value | ||
| } |
There was a problem hiding this comment.
Harden CSV escaping against formula injection.
User-controlled cells starting with =, +, -, or @ can execute formulas in spreadsheet apps when this CSV is opened.
🔒 Suggested fix
function escapeCsv(value: string): string {
- if (value.includes('"') || value.includes(",") || value.includes("\n")) {
- return `"${value.replace(/"/g, '""')}"`
+ const safe = /^[=+\-@]/.test(value) ? `'${value}` : value
+ if (safe.includes('"') || safe.includes(",") || safe.includes("\n")) {
+ return `"${safe.replace(/"/g, '""')}"`
}
- return value
+ return safe
}Also applies to: 17-20
🤖 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/export.ts` around lines 8 - 13, The escapeCsv function does not
protect against formula injection attacks where user input starting with =, +,
-, or @ characters can execute formulas when the CSV is opened in spreadsheet
applications. Modify the escapeCsv function to check if the value starts with
any of these dangerous characters and, if detected, prepend a single quote to
neutralize the formula execution. Apply this check before or after the existing
quote/comma/newline escaping logic to ensure all potentially malicious inputs
are properly neutralized.
| if (fields.length < 5) { | ||
| consola.warn(`Line ${i + 1}: skipping (expected 5 fields, got ${fields.length})`) | ||
| failed++ | ||
| continue | ||
| } | ||
|
|
||
| const [name, cycle, tagsStr, priceStr, currency] = fields | ||
|
|
There was a problem hiding this comment.
Reject rows unless they have exactly 5 fields.
Rows with extra columns are currently accepted and silently truncated by destructuring, which can import incorrect values without a hard failure.
✅ Suggested fix
- if (fields.length < 5) {
- consola.warn(`Line ${i + 1}: skipping (expected 5 fields, got ${fields.length})`)
+ if (fields.length !== 5) {
+ consola.warn(`Line ${i + 1}: skipping (expected 5 fields, got ${fields.length})`)
failed++
continue
}📝 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.
| if (fields.length < 5) { | |
| consola.warn(`Line ${i + 1}: skipping (expected 5 fields, got ${fields.length})`) | |
| failed++ | |
| continue | |
| } | |
| const [name, cycle, tagsStr, priceStr, currency] = fields | |
| if (fields.length !== 5) { | |
| consola.warn(`Line ${i + 1}: skipping (expected 5 fields, got ${fields.length})`) | |
| failed++ | |
| continue | |
| } | |
| const [name, cycle, tagsStr, priceStr, currency] = fields |
🤖 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/import-csv.ts` around lines 78 - 85, The condition checking the
number of fields in the CSV import function is too permissive. Currently it only
rejects rows with fewer than 5 fields using `fields.length < 5`, but this allows
rows with extra columns to pass through and get silently truncated during
destructuring. Change the condition to use `fields.length !== 5` instead to
ensure rows are rejected unless they have exactly 5 fields, which will prevent
incorrect values from being imported without explicit error handling.
| if (Object.keys(data.monthlyByTag).length > 0) { | ||
| consola.log("") | ||
| consola.log(pc.bold("Monthly by tag:")) | ||
| const sorted = Object.entries(data.monthlyByTag).sort((a, b) => b[1].monthly - a[1].monthly) | ||
| for (const [tag, info] of sorted) { | ||
| consola.log( | ||
| ` ${tag.padEnd(16)} ${formatPrice(Math.round(info.monthly), "USD")}/month (${info.count} sub${info.count > 1 ? "s" : ""})`, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: Tag totals mix currencies without conversion.
monthlyByTag aggregates subscription prices across different currencies without FX conversion, then displays the sum using USD formatting. For example, a "streaming" tag with Netflix ($15.99 USD) and Spotify (¥980 JPY) would show ¥995.99/month which is meaningless.
Either convert all values to a common currency or display per-currency breakdowns within each tag.
🐛 Proposed fix: Track tag totals by currency
export type SummaryData = {
totalCount: number
monthlyByCurrency: Record<string, number>
- monthlyByTag: Record<string, { count: number; monthly: number }>
+ monthlyByTag: Record<string, { count: number; monthlyCurrency: Record<string, number> }>
mostExpensive: SharedArgs | undefined
}
export function calcSummary(subs: SharedArgs[]): SummaryData {
const monthlyByCurrency: Record<string, number> = {}
- const monthlyByTag: Record<string, { count: number; monthly: number }> = {}
+ const monthlyByTag: Record<string, { count: number; monthlyCurrency: Record<string, number> }> = {}
for (const sub of subs) {
const monthly = sub.price * periodFactor(sub.cycle, "monthly")
monthlyByCurrency[sub.currency] = (monthlyByCurrency[sub.currency] ?? 0) + monthly
for (const tag of sub.tags) {
- if (!monthlyByTag[tag]) monthlyByTag[tag] = { count: 0, monthly: 0 }
+ if (!monthlyByTag[tag]) monthlyByTag[tag] = { count: 0, monthlyCurrency: {} }
monthlyByTag[tag].count++
- monthlyByTag[tag].monthly += monthly
+ monthlyByTag[tag].monthlyCurrency[sub.currency] = (monthlyByTag[tag].monthlyCurrency[sub.currency] ?? 0) + monthly
}
}🤖 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/payment.ts` around lines 237 - 246, The monthlyByTag aggregation
mixes currency values without conversion, causing incorrect totals when a single
tag contains subscriptions in different currencies. Refactor the monthlyByTag
data structure to track totals separately by currency for each tag, changing
from a flat monthly amount to a nested structure that groups monthly totals and
counts by currency. Then update the display loop that iterates over sorted
entries to handle the per-currency breakdown, displaying each currency
separately under its tag with the appropriate currency symbol instead of
assuming all values are in USD.
| test("ensurePricingCache returns null when fetch fails and no cache", async () => { | ||
| const originalFetch = globalThis.fetch | ||
| globalThis.fetch = async () => { throw new Error("Network error") } | ||
|
|
||
| // Reset module state by importing fresh | ||
| const pricing = await import("./pricing.ts") | ||
|
|
||
| // This should fail since there's no cache and fetch fails | ||
| // But the implementation has caching logic, so this may need env setup | ||
| // For now just verify it doesn't throw | ||
|
|
||
| globalThis.fetch = originalFetch | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '\bensurePricingCache\s*\(' subtrack/src/pricing.test.tsRepository: nazozokc/subtrack
Length of output: 126
🏁 Script executed:
sed -n '120,165p' subtrack/src/pricing.test.tsRepository: nazozokc/subtrack
Length of output: 1370
🏁 Script executed:
cat subtrack/src/pricing.ts | head -100Repository: nazozokc/subtrack
Length of output: 2985
Tests for ensurePricingCache lack function calls and assertions.
Both tests mock globalThis.fetch and import the module, but never invoke ensurePricingCache() or assert its output. The tests pass regardless of implementation correctness.
Per coding guidelines ("Write unit tests for all functions and critical code paths"), add actual function calls and assertions:
Suggested changes
test("ensurePricingCache returns null when fetch fails and no cache", async () => {
const originalFetch = globalThis.fetch
- globalThis.fetch = async () => { throw new Error("Network error") }
-
- // Reset module state by importing fresh
- const pricing = await import("./pricing.ts")
-
- // This should fail since there's no cache and fetch fails
- // But the implementation has caching logic, so this may need env setup
- // For now just verify it doesn't throw
-
- globalThis.fetch = originalFetch
+ try {
+ globalThis.fetch = async () => {
+ throw new Error("Network error")
+ }
+ vi.resetModules()
+ const { ensurePricingCache } = await import("./pricing.ts")
+ await expect(ensurePricingCache()).resolves.toBeNull()
+ } finally {
+ globalThis.fetch = originalFetch
+ }
})
test("ensurePricingCache parses GitHub JSON correctly", async () => {
const mockData: PricingCache = { ... }
const originalFetch = globalThis.fetch
- globalThis.fetch = async () =>
- new Response(JSON.stringify(mockData), {
- headers: { "Content-Type": "application/json" },
- })
-
- const pricing = await import("./pricing.ts")
- // This call will try to fetch, and we can't easily reset the cache state
- // Just verifying the function doesn't throw
-
- globalThis.fetch = originalFetch
+ try {
+ globalThis.fetch = async () =>
+ new Response(JSON.stringify(mockData), {
+ headers: { "Content-Type": "application/json" },
+ })
+ vi.resetModules()
+ const { ensurePricingCache } = await import("./pricing.ts")
+ await expect(ensurePricingCache()).resolves.toEqual(mockData)
+ } finally {
+ globalThis.fetch = originalFetch
+ }
})🤖 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/pricing.test.ts` around lines 126 - 138, In the test
"ensurePricingCache returns null when fetch fails and no cache", add an actual
invocation of the ensurePricingCache function from the imported pricing module
and include assertions to verify it returns null when fetch fails and no cache
exists. Currently the test mocks globalThis.fetch but never calls the
ensurePricingCache function or asserts its return value, rendering the test
unable to validate the function's actual behavior.
Source: Coding guidelines
| export function calculateCostCents( | ||
| pricing: ModelPricingEntry, | ||
| inputTokens: number, | ||
| outputTokens: number, | ||
| ): number { | ||
| const inputCost = (pricing.input_cost_per_token ?? 0) * inputTokens | ||
| const outputCost = (pricing.output_cost_per_token ?? 0) * outputTokens | ||
| const reasoningCost = | ||
| (pricing.output_cost_per_reasoning_token ?? 0) * outputTokens | ||
| return (inputCost + outputCost + reasoningCost) * 100 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
LiteLLM output_cost_per_reasoning_token vs output_cost_per_token usage
💡 Result:
In LiteLLM, output_cost_per_token and output_cost_per_reasoning_token serve distinct roles in cost calculation for models that support reasoning [1][2]. output_cost_per_token This parameter represents the cost per standard (non-reasoning) completion token [3][2]. It is the base configuration used for typical output text generated by a model [4]. output_cost_per_reasoning_token This parameter is specifically used for models that support "reasoning" or "thinking" tokens (e.g., OpenAI o1/o3 series, DeepSeek R1) [1][5][4]. It allows LiteLLM to apply a different, often higher, pricing tier to the reasoning portion of the output compared to the standard output tokens [6][4]. Key Usage Considerations: 1. Differential Billing: When both keys are defined, LiteLLM attempts to calculate costs by distinguishing between reasoning tokens and standard (text) completion tokens [4]. 2. Calculation Logic: LiteLLM's internal cost utility splits total completion tokens into categories (reasoning tokens vs. text/non-reasoning tokens) [4]. If a provider returns specific reasoning_tokens in the usage metadata, LiteLLM uses output_cost_per_reasoning_token for those tokens and output_cost_per_token for the remainder [4]. 3. Bug Fixes and Accuracy: Historical issues have occurred where LiteLLM miscalculated costs by ignoring non-reasoning tokens when reasoning tokens were present [5][4]. Current versions of LiteLLM correctly calculate costs by ensuring that any completion tokens not classified as reasoning (or other specialized types like audio/image) are billed at the standard output_cost_per_token rate [4]. 4. Custom Configuration: When registering custom model pricing, you can provide both parameters in the model information dictionary to ensure accurate spend tracking for models with specialized reasoning tiers [2][7]. Top results: [6][5][4][2]
Citations:
- 1: https://github.com/BerriAI/litellm/blob/main/docs/my-website/docs/proxy/custom_pricing.md
- 2: https://docs.litellm.ai/docs/proxy/custom_pricing
- 3: https://docs.litellm.ai/docs/completion/input
- 4: fix: correct cost calculation when reasoning_tokens are without text_tokens BerriAI/litellm#18607
- 5: [Bug]: Incorrect cost calculation when models are returning reasoning_tokens BerriAI/litellm#18599
- 6: [Bug]: Gemini Flash 2.5 cost calculation is incorrect BerriAI/litellm#11667
- 7: https://docs.litellm.ai/docs/provider_registration/add_model_pricing
🏁 Script executed:
cd subtrack && find . -type f -name "*.ts" -o -name "*.tsx" | head -20Repository: nazozokc/subtrack
Length of output: 336
🏁 Script executed:
cd subtrack && rg "calculateCostCents" -A 3 -B 3Repository: nazozokc/subtrack
Length of output: 3688
🏁 Script executed:
cd subtrack && rg "ModelPricingEntry" -A 5Repository: nazozokc/subtrack
Length of output: 3807
🏁 Script executed:
cd subtrack && cat -n src/usage.ts | sed -n '1,150p'Repository: nazozokc/subtrack
Length of output: 5391
🏁 Script executed:
cd subtrack && rg "outputTokens|reasoning" src/usage.ts -B 5 -A 5Repository: nazozokc/subtrack
Length of output: 1490
🏁 Script executed:
cd subtrack && cat -n src/pricing.test.ts | grep -A 15 "includes reasoning tokens"Repository: nazozokc/subtrack
Length of output: 762
🏁 Script executed:
cd subtrack && cat -n src/types.ts | grep -A 20 "LlmUsageEntry"Repository: nazozokc/subtrack
Length of output: 908
🏁 Script executed:
cd subtrack && rg "reasoning" src/ -iRepository: nazozokc/subtrack
Length of output: 524
🏁 Script executed:
cd subtrack && cat -n src/pricing.test.ts | sed -n '105,125p'Repository: nazozokc/subtrack
Length of output: 897
🏁 Script executed:
cd subtrack && cat -n src/pricing.ts | sed -n '140,155p'Repository: nazozokc/subtrack
Length of output: 719
Bug: Double-charging for output tokens when reasoning cost is defined.
The function charges both output_cost_per_token and output_cost_per_reasoning_token against the same outputTokens count. For models like o1 with both fields defined, this results in both rates being applied to every output token.
According to LiteLLM's model, these are meant for different token types: output_cost_per_token for standard completion tokens and output_cost_per_reasoning_token for reasoning-specific tokens tracked separately. The function needs either:
- A separate
reasoningTokensparameter to distinguish token types, or - Logic to treat them as mutually exclusive (use reasoning price if available, else standard price)
The current test validates the double-charging behavior and must be corrected accordingly.
🤖 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/pricing.ts` around lines 144 - 154, The calculateCostCents
function is applying both outputCost and reasoningCost to the same outputTokens
value, causing double-charging for output tokens. To fix this, either add a
separate reasoningTokens parameter to the function signature and use it for
calculating reasoningCost instead of outputTokens, or modify the reasoningCost
calculation to only apply when output_cost_per_reasoning_token is defined and
make it mutually exclusive with the standard output_cost_per_token calculation.
Additionally, ensure the corresponding test is updated to validate the corrected
behavior instead of the current double-charging scenario.
| // Date | ||
| let date: string | ||
| const today = new Date().toISOString().split("T")[0] | ||
| if (flags.date !== undefined) { | ||
| const result = validateDate(flags.date) | ||
| if (result !== true) { consola.error(result); return null } | ||
| date = flags.date | ||
| } else { | ||
| date = today | ||
| } |
There was a problem hiding this comment.
Empty date string passes validation but is stored as-is instead of defaulting to today.
When flags.date is an empty string "", validateDate("") returns true (since !v.trim() is truthy), but then date = flags.date stores the empty string. This creates an entry with date = "" instead of today's date.
🐛 Proposed fix
// Date
let date: string
const today = new Date().toISOString().split("T")[0]
if (flags.date !== undefined) {
const result = validateDate(flags.date)
if (result !== true) { consola.error(result); return null }
- date = flags.date
+ date = flags.date.trim() || today
} else {
date = today
}📝 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.
| // Date | |
| let date: string | |
| const today = new Date().toISOString().split("T")[0] | |
| if (flags.date !== undefined) { | |
| const result = validateDate(flags.date) | |
| if (result !== true) { consola.error(result); return null } | |
| date = flags.date | |
| } else { | |
| date = today | |
| } | |
| // Date | |
| let date: string | |
| const today = new Date().toISOString().split("T")[0] | |
| if (flags.date !== undefined) { | |
| const result = validateDate(flags.date) | |
| if (result !== true) { consola.error(result); return null } | |
| date = flags.date.trim() || today | |
| } else { | |
| date = today | |
| } |
🤖 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/usage.ts` around lines 88 - 97, The validation function
validateDate returns true for empty strings because !v.trim() evaluates to true
when the trimmed value is empty, but then the empty string gets assigned to the
date variable instead of defaulting to today. After the validateDate check in
the conditional block where flags.date is validated, add an additional check to
ensure the trimmed date value is not empty; if it is empty after trimming, set
date to today instead of storing the empty string. Alternatively, modify the
conditional logic to check that flags.date is not only defined but also has a
non-empty trimmed value before proceeding with validation.
1 participant
概要
npm パッケージ subtrack のサプライチェーンセキュリティを強化し、Socket.dev スコアを 75 → 100 に改善する。
変更内容
Package metadata（Socket.dev Quality スコア向上）
description,keywords,homepage,bugsを package.json に追加engines.node >=22で Node.js バージョンを明示sideEffects: falseでバンドラ最適化を許可Supply chain security（サプライチェーン攻撃対策）
.npmrc:engine-strict=true,strict-peer-dependencies=truepnpm-workspace.yaml:onlyBuiltDependenciesで esbuild のみ許可（postinstall スクリプト制限）osvVulnerabilityAlerts有効化、npm:unpublishSafe追加pnpm auditを実施Socket.dev スコア改善の根拠
確認事項
pnpm test全 175 tests passpnpm build成功pnpm install --frozen-lockfile正常Summary by CodeRabbit
Release Notes
New Features
usagecommand group for adding, listing, deleting, and refreshing usage entries.--apiflag to show API usage statistics and costs.Improvements