feat(kimi): implement Kimi CLI usage tracking#840
feat(kimi): implement Kimi CLI usage tracking#840azidancorp wants to merge 7 commits intoryoppippi:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant Cmd as Kimi Command
participant Loader as Data Loader
participant Wire as wire.jsonl Files
participant Builder as Report Builder
participant Pricing as KimiPricingSource
participant Output as Table/JSON
User->>Cmd: run command (e.g. daily --json --since ...)
Cmd->>Loader: loadTokenUsageEvents(shareDir?)
Loader->>Wire: read & stream parse wire.jsonl
Wire-->>Loader: lines (ContentPart / StatusUpdate)
Loader->>Loader: dedupe, extract reasoning, map workdirs
Loader-->>Cmd: TokenUsageEvent[]
Cmd->>Builder: buildReport(events, { pricingSource, ... })
Builder->>Pricing: getPricing(model) for each model
Pricing-->>Builder: ModelPricing
Builder-->>Cmd: ReportRow[]
Cmd->>Output: render JSON or table
Output-->>User: display results
sequenceDiagram
participant MCPClient as MCP Client
participant MCPServer as MCP Server
participant Wrapper as Kimi Wrapper (apps/mcp/src/kimi.ts)
participant KimiCLI as Kimi CLI Process
participant Zod as Zod Validator
MCPClient->>MCPServer: invoke tool (kimi-daily/etc.)
MCPServer->>Wrapper: getKimiDaily(params)
Wrapper->>Zod: validate params
Wrapper->>KimiCLI: spawn ccusage-kimi --json ...
KimiCLI->>KimiCLI: load files, build report, print JSON
KimiCLI-->>Wrapper: JSON output
Wrapper->>Zod: parse & validate output
Wrapper-->>MCPServer: return typed result
MCPServer-->>MCPClient: tool response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/mcp/package.json (1)
55-66: 🛠️ Refactor suggestion | 🟠 MajorMove
@ccusage/kimitodevDependencies.Per coding guidelines, all projects under
apps/ship as bundled CLIs — runtime dependencies should be listed indevDependencies(neverdependencies) so the bundler owns the runtime payload.Proposed fix
"dependencies": { "@ccusage/codex": "workspace:*", - "@ccusage/kimi": "workspace:*", "@hono/mcp": "catalog:runtime", ... }, "devDependencies": { "@ccusage/internal": "workspace:*", + "@ccusage/kimi": "workspace:*", "@ryoppippi/eslint-config": "catalog:lint", ... },As per coding guidelines: "All projects under
apps/ship as bundled CLIs/binaries - list runtime dependencies indevDependencies(neverdependencies) so the bundler owns the runtime payload".
🤖 Fix all issues with AI agents
In `@apps/ccusage/README.md`:
- Around line 36-38: Update the README blurb under the "### 🌙 [`@ccusage/kimi`]"
header to list "weekly" alongside "daily, monthly, and session-based" reports;
specifically, edit the sentence that currently reads "Track daily, monthly, and
session-based token usage and costs from your local `~/.kimi/sessions` logs." to
include "weekly" (and optionally reference the provided `kimi-weekly` tool) so
it reads something like "Track daily, weekly, monthly, and session-based token
usage and costs from your local `~/.kimi/sessions` logs." Ensure the change is
made in the same section containing the [`@ccusage/kimi`] heading and the
backticked path `~/.kimi/sessions`.
In `@apps/kimi/CLAUDE.md`:
- Line 150: The sentence in CLAUDE.md is self-contradictory; update the line
that reads "No weekly command: Implemented (unlike other apps)" to correctly
state that the weekly command is implemented (e.g., "Weekly command: Implemented
(unlike other apps)"), reflecting the presence of the weekly.ts command in the
codebase.
In `@apps/kimi/README.md`:
- Around line 121-125: The sponsors SVG image tag in the README (<img
src="https://cdn.jsdelivr.net/gh/ryoppippi/sponsors@main/sponsors.svg">) is
missing an alt attribute; update that <img> element to include a descriptive alt
text (e.g., alt="Project sponsors" or similar) to satisfy accessibility
guidelines and ensure screen readers convey the image purpose.
In `@apps/kimi/src/_consts.ts`:
- Around line 12-13: SESSION_GLOB is constructed incorrectly and expands to a
two-level pattern; change its definition so it matches one-level session
directories. Replace the current SESSION_GLOB = `*/${WIRE_GLOB}` with a pattern
that directly uses KIMI_WIRE_FILE_NAME (e.g., SESSION_GLOB =
`*/${KIMI_WIRE_FILE_NAME}`) or set SESSION_GLOB to the same value as WIRE_GLOB
so glob(cwd: sessionsDir) will correctly match files like
~/.kimi/sessions/{session_id}/wire.jsonl; update the constant in
apps/kimi/src/_consts.ts where WIRE_GLOB, SESSION_GLOB, and KIMI_WIRE_FILE_NAME
are defined.
🧹 Nitpick comments (10)
apps/kimi/src/command-utils.ts (1)
3-8: Type inconsistency:reasoningOutputTokensnullish coalescing vs required type.Line 7 declares
reasoningOutputTokens: numberas required, but line 19 usesusage.reasoningOutputTokens ?? 0, implying it could be undefined. Either make the type optional (reasoningOutputTokens?: number) or remove the nullish coalescing if the field is always present.♻️ Option A: Make type optional
export type UsageGroup = { inputTokens: number; cachedInputTokens: number; outputTokens: number; - reasoningOutputTokens: number; + reasoningOutputTokens?: number; };♻️ Option B: Remove nullish coalescing
- const rawReasoning = usage.reasoningOutputTokens ?? 0; + const rawReasoning = usage.reasoningOutputTokens;Also applies to: 19-19
apps/kimi/src/token-utils.ts (1)
15-21: Consider documenting the mutation behavior.
addUsagemutatestargetin place rather than returning a new object. This is a valid pattern for performance, but a brief JSDoc comment would clarify intent for future maintainers.apps/kimi/src/commands/monthly.ts (1)
36-42: Consider preserving error details in logging.
String(error)may lose stack trace information. For better debugging, consider logging the error object directly or checking if it's an Error instance.♻️ Proposed improvement
} catch (error) { - logger.error(String(error)); + logger.error(error instanceof Error ? error.message : String(error)); process.exit(1); }apps/kimi/CLAUDE.md (2)
49-55: Add language specifier to fenced code block.The markdownlint tool flags this code block for missing a language specifier. Consider adding a language identifier (e.g.,
textorplaintext) to satisfy MD040.Suggested fix
-``` +```text inputTokens = input_other + input_cache_read + input_cache_creation
117-119: Add language specifier to fenced code block.Same markdownlint MD040 issue - add a language identifier.
Suggested fix
-``` +```text {work_directory_path}/{session_file_id}apps/kimi/src/commands/session.ts (1)
175-177: Missing compact mode notice.The daily command (lines 166-169 in daily.ts) displays a helpful notice when running in compact mode, informing users to expand terminal width for additional metrics. This session command lacks that notice, creating inconsistent UX.
Suggested fix
log(table.toString()); + + if (table.isCompactMode()) { + logger.info('\nRunning in Compact Mode'); + logger.info('Expand terminal width to see reasoning and cache metrics'); + } },apps/kimi/src/daily-report.ts (1)
83-86: Consider parallelizing pricing lookups.The sequential
awaitin the loop could be parallelized for better performance when there are multiple unique models. While the impact is likely minimal (few unique models), it's a common optimization pattern.Suggested fix
const modelPricing = new Map<string, Awaited<ReturnType<PricingSource['getPricing']>>>(); -for (const modelName of uniqueModels) { - modelPricing.set(modelName, await pricingSource.getPricing(modelName)); -} +await Promise.all( + Array.from(uniqueModels).map(async (modelName) => { + modelPricing.set(modelName, await pricingSource.getPricing(modelName)); + }), +);apps/kimi/src/pricing.ts (2)
4-4: Module-levelwarnedModelsstate may affect test isolation.The
warnedModelsSet persists across allKimiPricingSourceinstances and test runs within the same process. While this correctly deduplicates warnings in production, it means:
- Tests cannot verify the warning is emitted without mocking the logger
- If a test queries an unknown model, subsequent tests won't see the warning for that same model
Consider either:
- Accepting this as intended behavior (warnings are one-time per process)
- Adding a
clearWarnings()method for testing purposes (unexported)This is a minor concern since the current test uses a unique model name and the behavior is documented.
Also applies to: 65-70
12-15: IIFE forlastSegmentextraction is unconventional.The immediately-invoked function expression (IIFE) works but is unusual for this simple operation. A more idiomatic approach would be direct assignment.
Suggested simplification
-const lastSegment = (() => { - const idx = trimmed.lastIndexOf('/'); - return idx >= 0 ? trimmed.slice(idx + 1) : trimmed; -})(); +const idx = trimmed.lastIndexOf('/'); +const lastSegment = idx >= 0 ? trimmed.slice(idx + 1) : trimmed;apps/mcp/src/kimi.ts (1)
145-163: Wrap JSON parsing withResult.try()for consistent error handling.
JSON.parsecan throw; using@praha/byethrowkeeps failures consistent with project guidelines.♻️ Suggested refactor
-import { z } from 'zod'; +import { Result } from '@praha/byethrow'; +import { z } from 'zod'; @@ const kimiWeeklyResponseSchema = z.object({ weekly: z.array(kimiWeeklyRowSchema), totals: kimiTotalsSchema.nullable(), }); + +function parseKimiJson<T>(raw: string, schema: z.ZodType<T>): T { + const parsed = Result.try({ + try: () => JSON.parse(raw) as unknown, + catch: (error) => error, + })(); + if (Result.isFailure(parsed)) { + throw parsed.error; + } + return schema.parse(parsed.value); +} @@ export async function getKimiDaily(parameters: z.infer<typeof kimiParametersSchema>) { const raw = await runKimiCliJson('daily', parameters); - return kimiDailyResponseSchema.parse(JSON.parse(raw)); + return parseKimiJson(raw, kimiDailyResponseSchema); } @@ export async function getKimiMonthly(parameters: z.infer<typeof kimiParametersSchema>) { const raw = await runKimiCliJson('monthly', parameters); - return kimiMonthlyResponseSchema.parse(JSON.parse(raw)); + return parseKimiJson(raw, kimiMonthlyResponseSchema); } @@ export async function getKimiSession(parameters: z.infer<typeof kimiParametersSchema>) { const raw = await runKimiCliJson('session', parameters); - return kimiSessionResponseSchema.parse(JSON.parse(raw)); + return parseKimiJson(raw, kimiSessionResponseSchema); } @@ export async function getKimiWeekly(parameters: z.infer<typeof kimiParametersSchema>) { const raw = await runKimiCliJson('weekly', parameters); - return kimiWeeklyResponseSchema.parse(JSON.parse(raw)); + return parseKimiJson(raw, kimiWeeklyResponseSchema); }
| export const WIRE_GLOB = `*/${KIMI_WIRE_FILE_NAME}`; | ||
| export const SESSION_GLOB = `*/${WIRE_GLOB}`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "SESSION_GLOB|WIRE_GLOB" --type ts -C 2Repository: ryoppippi/ccusage
Length of output: 2044
🏁 Script executed:
# Also check the directory structure for sessions and wires
find . -type f -name "*.jsonl" | head -20Repository: ryoppippi/ccusage
Length of output: 102
🏁 Script executed:
# Check if there are any test files or fixtures that show the expected structure
find . -type d -name "*session*" -o -type d -name "*wire*" | head -20Repository: ryoppippi/ccusage
Length of output: 43
🏁 Script executed:
# Check data-loader.ts for context on expected directory structure
sed -n '160,180p' apps/kimi/src/data-loader.tsRepository: ryoppippi/ccusage
Length of output: 749
🏁 Script executed:
# Search for any comments or documentation about the directory structure
rg -i "session.*structure|directory.*structure|wire.*path" apps/kimi/src/ --type tsRepository: ryoppippi/ccusage
Length of output: 43
🏁 Script executed:
# Check if there are any tests or fixtures that show the expected structure
find apps/kimi -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "fixture*" \) | head -10Repository: ryoppippi/ccusage
Length of output: 43
🏁 Script executed:
# Check how sessions are created/structured by looking for mkdir or path.join patterns
rg -n "sessions|session-" apps/kimi/src/ --type ts -B 2 -A 2 | head -40Repository: ryoppippi/ccusage
Length of output: 2291
🏁 Script executed:
# Look for any README or documentation about the data structure
find apps/kimi -type f \( -name "README*" -o -name "*.md" \)Repository: ryoppippi/ccusage
Length of output: 100
🏁 Script executed:
# Check if the pattern actually works by looking at what files are scanned
rg -n "loadKimiEvents\|loadEvents" apps/kimi/src/ --type ts -A 5 | head -30Repository: ryoppippi/ccusage
Length of output: 43
🏁 Script executed:
# Check the README for directory structure information
cat apps/kimi/README.mdRepository: ryoppippi/ccusage
Length of output: 6719
🏁 Script executed:
# Also check CLAUDE.md
cat apps/kimi/CLAUDE.mdRepository: ryoppippi/ccusage
Length of output: 6697
SESSION_GLOB pattern is incorrect and will not match session files.
The current definition SESSION_GLOB = \/${WIRE_GLOB}`expands to//wire.jsonl, but session files are actually located at ~/.kimi/sessions/{session_id}/wire.jsonl(one level deep). Whenglob()is called withcwd: sessionsDir, the pattern /*/wire.jsonl` looks for files two levels deep and will not match any files.
Change SESSION_GLOB to match the actual structure: */${KIMI_WIRE_FILE_NAME} (same as WIRE_GLOB) or restructure the definition to avoid string interpolation issues.
🤖 Prompt for AI Agents
In `@apps/kimi/src/_consts.ts` around lines 12 - 13, SESSION_GLOB is constructed
incorrectly and expands to a two-level pattern; change its definition so it
matches one-level session directories. Replace the current SESSION_GLOB =
`*/${WIRE_GLOB}` with a pattern that directly uses KIMI_WIRE_FILE_NAME (e.g.,
SESSION_GLOB = `*/${KIMI_WIRE_FILE_NAME}`) or set SESSION_GLOB to the same value
as WIRE_GLOB so glob(cwd: sessionsDir) will correctly match files like
~/.kimi/sessions/{session_id}/wire.jsonl; update the constant in
apps/kimi/src/_consts.ts where WIRE_GLOB, SESSION_GLOB, and KIMI_WIRE_FILE_NAME
are defined.
There was a problem hiding this comment.
This seems to be inaccurate - this is the actual path structure on my current .kimi:
~/.kimi/sessions/138fb6b4331c5ba58fbc2498965ac4c7/7781ea1b-3dd7-4348-9b87-70243ed62887$ ls context.jsonl wire.jsonl
Also checked with three agents which agree its a false positive
There was a problem hiding this comment.
@azidancorp, you're absolutely right—I apologize for the false positive!
The pattern SESSION_GLOB = */*/wire.jsonl is indeed correct for the actual directory structure. When glob runs with cwd: sessionsDir (~/.kimi/sessions), it needs to match:
{hash}/{session_id}/wire.jsonl
which is exactly two directory levels, so */*/wire.jsonl matches perfectly.
Thank you for verifying with the actual filesystem and multiple agents—I appreciate the thorough validation!
🧠 Learnings used
Learnt from: CR
Repo: ryoppippi/ccusage PR: 0
File: apps/opencode/CLAUDE.md:0-0
Timestamp: 2026-01-09T11:07:17.625Z
Learning: Applies to apps/opencode/**/*.{ts,tsx,js,jsx} : Data discovery relies on `OPENCODE_DATA_DIR` environment variable with default path `~/.local/share/opencode`
Learnt from: CR
Repo: ryoppippi/ccusage PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T12:22:39.548Z
Learning: Applies to **/*.ts : Only export constants, functions, and types that are actually used by other modules - do not export internal/private constants used only within the same file
Learnt from: CR
Repo: ryoppippi/ccusage PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T12:22:39.548Z
Learning: Applies to **/*.ts : Use UPPER_SNAKE_CASE for constants (e.g., `DEFAULT_CLAUDE_CODE_PATH`)
Learnt from: CR
Repo: ryoppippi/ccusage PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T12:22:39.548Z
Learning: For apps, use the app directory name in commit scope (e.g., `feat(ccusage):`, `fix(mcp):`)
Learnt from: CR
Repo: ryoppippi/ccusage PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T12:22:39.548Z
Learning: For packages, use the package directory name in commit scope (e.g., `feat(terminal):`, `fix(ui):`)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/kimi/CLAUDE.md`:
- Around line 138-141: The three consecutive bullets in CLAUDE.md start with the
same phrase "Tests verify …", which reduces readability; update one or two of
them to vary the phrasing while preserving meaning—e.g., change the second
bullet ("Tests verify token field mapping from Kimi's wire format") to "Verifies
token field mapping from Kimi's wire format" and the third ("Tests verify work
directory hash mapping and deduplication logic") to "Checks work directory hash
mapping and deduplication logic", keeping the first bullet unchanged; ensure the
wording still accurately reflects the tests' intent.
🧹 Nitpick comments (1)
apps/kimi/README.md (1)
2-2: Use relative paths for images stored in/docs/public/.These two images are served from
docs/publicbut use absolute CDN URLs. Per the markdown guideline, switch to relative paths.
As per coding guidelines: "Use relative image paths like/screenshot.pngfor images stored in/docs/public/."🔧 Suggested fix
- <img src="https://cdn.jsdelivr.net/gh/ryoppippi/ccusage@main/docs/public/logo.svg" alt="ccusage logo" width="256" height="256"> + <img src="/logo.svg" alt="ccusage logo" width="256" height="256">- <img src="https://cdn.jsdelivr.net/gh/ryoppippi/ccusage@main/docs/public/ccusage_thumbnail.png" alt="ccusage: The Claude Code cost scorecard that went viral" width="600"> + <img src="/ccusage_thumbnail.png" alt="ccusage: The Claude Code cost scorecard that went viral" width="600">Also applies to: 117-117
| - Tests rely on `fs-fixture` with `using` to ensure cleanup. | ||
| - Tests verify token field mapping from Kimi's wire format. | ||
| - Tests verify work directory hash mapping and deduplication logic. | ||
| - Tests verify pricing lookup for known models and fallback for unknown models. |
There was a problem hiding this comment.
Reduce repeated “Tests verify …” sentence starts for readability.
These three bullets begin identically; rephrase one or two to improve flow.
🧰 Tools
🪛 LanguageTool
[style] ~140-~140: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ield mapping from Kimi's wire format. - Tests verify work directory hash mapping and ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~141-~141: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...hash mapping and deduplication logic. - Tests verify pricing lookup for known models ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
In `@apps/kimi/CLAUDE.md` around lines 138 - 141, The three consecutive bullets in
CLAUDE.md start with the same phrase "Tests verify …", which reduces
readability; update one or two of them to vary the phrasing while preserving
meaning—e.g., change the second bullet ("Tests verify token field mapping from
Kimi's wire format") to "Verifies token field mapping from Kimi's wire format"
and the third ("Tests verify work directory hash mapping and deduplication
logic") to "Checks work directory hash mapping and deduplication logic", keeping
the first bullet unchanged; ensure the wording still accurately reflects the
tests' intent.
There was a problem hiding this comment.
Seems fine as is, leaving this one.
Ive added the three other suggested commits, and rejected the false positive for wire.json paths
There was a problem hiding this comment.
@azidancorp, understood! Thanks for addressing the other suggestions. 👍
🧠 Learnings used
Learnt from: CR
Repo: ryoppippi/ccusage PR: 0
File: apps/ccusage/CLAUDE.md:0-0
Timestamp: 2025-09-18T16:06:37.474Z
Learning: Applies to apps/ccusage/src/**/*.ts : In tests, use current Claude 4 models (sonnet-4, opus-4)
Learnt from: CR
Repo: ryoppippi/ccusage PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T12:22:39.548Z
Learning: The data loading architecture follows a clear separation of concerns: Data Loading (data-loader.ts), Token Aggregation (calculate-cost.ts), Command Execution (commands/), and CLI Entry (index.ts)
Learnt from: CR
Repo: ryoppippi/ccusage PR: 0
File: apps/ccusage/CLAUDE.md:0-0
Timestamp: 2025-09-18T16:06:37.474Z
Learning: Applies to apps/ccusage/src/**/*.ts : Use `fs-fixture` with `createFixture()` to simulate Claude data in tests
Learnt from: CR
Repo: ryoppippi/ccusage PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T12:22:39.548Z
Learning: For apps, use the app directory name in commit scope (e.g., `feat(ccusage):`, `fix(mcp):`)
Learnt from: CR
Repo: ryoppippi/ccusage PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T12:22:39.548Z
Learning: For packages, use the package directory name in commit scope (e.g., `feat(terminal):`, `fix(ui):`)
|
I havent registered or published anything on npm for this, link in the readme wont work until then |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/kimi/README.md`:
- Around line 7-8: The README currently includes an npm badge/link for
`@ccusage/kimi` which will 404 until the package is published; update the
README.md to either remove the npm badge/link lines referencing `@ccusage/kimi` or
replace them with a “Coming soon — package not yet published” note (leave the
existing socket.dev badge if desired) so users aren’t led to a broken npm page;
look for the badge markup containing "@ccusage/kimi" to locate and change the
lines.
| <a href="https://socket.dev/api/npm/package/@ccusage/kimi"><img src="https://socket.dev/api/badge/npm/package/@ccusage/kimi" alt="Socket Badge" /></a> | ||
| <a href="https://npmjs.com/package/@ccusage/kimi"><img src="https://img.shields.io/npm/v/@ccusage/kimi?color=yellow" alt="npm version" /></a> |
There was a problem hiding this comment.
NPM badge/link will be broken until the package is published.
Given the package isn’t registered yet, the badge and link will likely 404. Consider adding a “coming soon” note or removing the npm badge/link until publication to avoid confusion.
🤖 Prompt for AI Agents
In `@apps/kimi/README.md` around lines 7 - 8, The README currently includes an npm
badge/link for `@ccusage/kimi` which will 404 until the package is published;
update the README.md to either remove the npm badge/link lines referencing
`@ccusage/kimi` or replace them with a “Coming soon — package not yet published”
note (leave the existing socket.dev badge if desired) so users aren’t led to a
broken npm page; look for the badge markup containing "@ccusage/kimi" to locate
and change the lines.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/kimi/src/data-loader.ts (1)
7-7: Consider moving test-only import inside the vitest block.
createFixtureis only used within theimport.meta.vitestblock but is imported at module top-level. While tree-shaking should handle this in production builds, moving it inside the test block makes the test dependency scope explicit:if (import.meta.vitest != null) { const { createFixture } = await import('fs-fixture'); // ...tests }However, given the coding guideline against dynamic imports, the current approach is acceptable if the bundler handles tree-shaking correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/kimi/src/data-loader.ts` at line 7, The top-level import of createFixture should be moved into the test-only block so the test dependency is scoped to import.meta.vitest: remove the top-level "import { createFixture } from 'fs-fixture';" and inside the "if (import.meta.vitest != null) { ... }" block dynamically import it (e.g. const { createFixture } = await import('fs-fixture');) and use that local symbol for tests; if your project forbids dynamic imports, leave the top-level import but add a comment explaining it's test-only and relies on bundler tree-shaking.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/kimi/src/data-loader.ts`:
- Around line 649-651: Update the misleading comment near the thinkContent and
expectedReasoningTokens declarations: the string in thinkContent is 65
characters, not "~40 chars", so correct the comment to reflect the actual length
(or remove the approximate char count) and clarify token estimation (e.g., "65
chars ≈ 17 tokens at 4 chars/token"), keeping the Math.ceil(thinkContent.length
/ 4) calculation unchanged.
- Around line 125-133: The toIsoTimestamp function currently calls
date.toISOString() which will throw for invalid dates; change it to validate the
Date before calling toISOString: after creating const date = new Date(ms), check
Number.isFinite(date.getTime()) (or !Number.isNaN(date.getTime())) and return
null if false, then safely call date.toISOString() and return it; update the
function to remove the iso === 'Invalid Date' check and rely on the pre-check to
avoid the RangeError in toIsoTimestamp.
- Around line 227-230: The endsWith check in the for loop over files fails on
Windows because tinyglobby returns POSIX-style paths; update the condition that
uses path.sep so it works cross-platform—either compare using a forward slash
like file.endsWith(`/${KIMI_WIRE_FILE_NAME}`) or, more robustly, use
path.basename(file) === KIMI_WIRE_FILE_NAME (referencing the for (const file of
files) loop and KIMI_WIRE_FILE_NAME) to locate the wire file regardless of path
separator.
---
Nitpick comments:
In `@apps/kimi/src/data-loader.ts`:
- Line 7: The top-level import of createFixture should be moved into the
test-only block so the test dependency is scoped to import.meta.vitest: remove
the top-level "import { createFixture } from 'fs-fixture';" and inside the "if
(import.meta.vitest != null) { ... }" block dynamically import it (e.g. const {
createFixture } = await import('fs-fixture');) and use that local symbol for
tests; if your project forbids dynamic imports, leave the top-level import but
add a comment explaining it's test-only and relies on bundler tree-shaking.
| function toIsoTimestamp(seconds: number): string | null { | ||
| const ms = seconds * 1000; | ||
| if (!Number.isFinite(ms)) { | ||
| return null; | ||
| } | ||
| const date = new Date(ms); | ||
| const iso = date.toISOString(); | ||
| return iso === 'Invalid Date' ? null : iso; | ||
| } |
There was a problem hiding this comment.
Bug: toISOString() throws on invalid dates, doesn't return 'Invalid Date'.
Date.prototype.toISOString() throws a RangeError for invalid dates rather than returning the string 'Invalid Date'. The 'Invalid Date' string is what toString() returns. This means the check on line 132 will never catch invalid dates—instead, the function will throw an uncaught exception.
🐛 Proposed fix to validate before calling toISOString
function toIsoTimestamp(seconds: number): string | null {
const ms = seconds * 1000;
if (!Number.isFinite(ms)) {
return null;
}
const date = new Date(ms);
- const iso = date.toISOString();
- return iso === 'Invalid Date' ? null : iso;
+ if (Number.isNaN(date.getTime())) {
+ return null;
+ }
+ return date.toISOString();
}📝 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.
| function toIsoTimestamp(seconds: number): string | null { | |
| const ms = seconds * 1000; | |
| if (!Number.isFinite(ms)) { | |
| return null; | |
| } | |
| const date = new Date(ms); | |
| const iso = date.toISOString(); | |
| return iso === 'Invalid Date' ? null : iso; | |
| } | |
| function toIsoTimestamp(seconds: number): string | null { | |
| const ms = seconds * 1000; | |
| if (!Number.isFinite(ms)) { | |
| return null; | |
| } | |
| const date = new Date(ms); | |
| if (Number.isNaN(date.getTime())) { | |
| return null; | |
| } | |
| return date.toISOString(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/kimi/src/data-loader.ts` around lines 125 - 133, The toIsoTimestamp
function currently calls date.toISOString() which will throw for invalid dates;
change it to validate the Date before calling toISOString: after creating const
date = new Date(ms), check Number.isFinite(date.getTime()) (or
!Number.isNaN(date.getTime())) and return null if false, then safely call
date.toISOString() and return it; update the function to remove the iso ===
'Invalid Date' check and rely on the pre-check to avoid the RangeError in
toIsoTimestamp.
| for (const file of files) { | ||
| if (!file.endsWith(`${path.sep}${KIMI_WIRE_FILE_NAME}`)) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and examine the actual code
find . -name "data-loader.ts" -type f | head -5Repository: ryoppippi/ccusage
Length of output: 222
🏁 Script executed:
# Check the exact code at lines 227-230
head -230 apps/kimi/src/data-loader.ts | tail -10Repository: ryoppippi/ccusage
Length of output: 202
🏁 Script executed:
# Look for tinyglobby usage and documentation
fd -t f "package.json" | xargs rg -l "tinyglobby" 2>/dev/null | head -3Repository: ryoppippi/ccusage
Length of output: 126
🏁 Script executed:
# Check tinyglobby version and behavior details
cat apps/kimi/package.json 2>/dev/null | rg "tinyglobby|version" -A1 -B1Repository: ryoppippi/ccusage
Length of output: 411
🏁 Script executed:
# Look at how glob is used in the context around line 227
sed -n '140,240p' apps/kimi/src/data-loader.ts | cat -nRepository: ryoppippi/ccusage
Length of output: 3799
🏁 Script executed:
# Search for tinyglobby in node_modules to check its behavior
find node_modules/tinyglobby -name "*.md" -o -name "*.d.ts" | head -10Repository: ryoppippi/ccusage
Length of output: 119
🏁 Script executed:
# Check tinyglobby's type definitions or README
cat node_modules/tinyglobby/README.md 2>/dev/null || cat node_modules/tinyglobby/package.json 2>/dev/null | grep -A5 -B5 "path\|posix\|separator"Repository: ryoppippi/ccusage
Length of output: 43
🏁 Script executed:
# Search for other uses of glob() in the codebase to see if there are any patterns
rg "glob\(" apps/kimi/src/ -A3 -B1Repository: ryoppippi/ccusage
Length of output: 291
🏁 Script executed:
# Check if there are any tests for data-loader.ts
find . -name "*data-loader*.test.ts" -o -name "*data-loader*.spec.ts" | grep kimiRepository: ryoppippi/ccusage
Length of output: 43
🏁 Script executed:
# Let's check the constant definitions for SESSION_GLOB and KIMI_WIRE_FILE_NAME
rg "SESSION_GLOB|KIMI_WIRE_FILE_NAME" apps/kimi/src/data-loader.ts -B2 -A2Repository: ryoppippi/ccusage
Length of output: 455
🏁 Script executed:
# Check imports to understand where glob comes from
head -30 apps/kimi/src/data-loader.ts | grep -E "import|from"Repository: ryoppippi/ccusage
Length of output: 564
🌐 Web query:
tinyglobby glob absolute path normalization posix windows
💡 Result:
tinyglobbywon’t “switch to absolute output” just because your pattern is absolute. It always returns results in a consistent format, and you must enable{ absolute: true }to get absolute paths. Otherwise you’ll get paths relative tocwd(default:process.cwd()). [1][2]- On Windows, write glob patterns using forward slashes (
/), not backslashes (\). Backslashes are treated as escapes in glob patterns, and patterns with\can fail silently. Use POSIX-style joins or a converter. [3] - If you’re starting from a real filesystem path (especially an absolute Windows path) and want a safe glob pattern, use
tinyglobby’sconvertPathToPattern(path). It’s designed for “path → glob pattern” conversion/escaping in a platform-aware way (and is the same idea recommended by other glob libs for Windows). [1][3] - Best-practice normalization workflow (cross-platform):
- Set
cwdto the directory you’re searching to avoid surprises and improve performance. [1] - Build patterns with
/separators (eg,path.posix.join(...)), or run the base path throughconvertPathToPattern(). [1][3] - Request absolute output explicitly with
{ absolute: true }if you need it. [1][2]
- Set
Sources: tinyglobby docs/options and convertPathToPattern [1], tinyglobby library comparison note about absolute behavior/consistent output [2], globby (glob patterns must use / on Windows; recommends convertPathToPattern / path.posix.join) [3].
🏁 Script executed:
# Let's check the _consts.ts file to see what SESSION_GLOB and KIMI_WIRE_FILE_NAME are
cat apps/kimi/src/_consts.ts | grep -A2 -B2 "SESSION_GLOB\|KIMI_WIRE_FILE_NAME"Repository: ryoppippi/ccusage
Length of output: 521
🏁 Script executed:
# Let's search for any documentation or tests in the repo about glob output format
rg "glob.*absolute" --type ts -A5 -B5 apps/ | head -50Repository: ryoppippi/ccusage
Length of output: 43
🌐 Web query:
tinyglobby absolute true output path format windows
💡 Result:
On Windows, tinyglobby always returns paths using forward slashes (/), even when absolute: true.
absolute: truemakes results absolute (resolved) paths (e.g.C:/project/src/file.ts).tinyglobbynormalizescwdby replacing\with/and configures its crawler (fdir) withpathSeparator: "/", so output stays/-separated on Windows. [1]- Supplying an absolute pattern does not make outputs absolute by itself—you must set the
absoluteoption. [2] - The
absoluteoption is explicitly “whether to return absolute paths”. [3]
If you need native Windows backslashes, convert after globbing (e.g., result.replaceAll("/", "\\") or path.win32.normalize(result)).
Sources:
[1] tinyglobby compiled source showing cwd normalization and pathSeparator: "/" (UNPKG) (app.unpkg.com)
[2] tinyglobby “Format” behavior (absolute pattern vs absolute option) (superchupu.dev)
[3] tinyglobby option docs for absolute (superchupu.dev)
Citations:
- 1: https://app.unpkg.com/tinyglobby%400.2.5/files/dist/index.mjs
- 2: https://superchupu.dev/tinyglobby/comparison
- 3: https://superchupu.dev/tinyglobby/documentation
Cross-platform bug: path.sep does not match tinyglobby output on Windows.
tinyglobby's glob() function always returns paths with forward slash (/) separators on all platforms, including Windows. Using path.sep (which is \ on Windows) will cause this check to fail on Windows, preventing the file from being identified and processed correctly.
🐛 Proposed fix to use forward slash consistently
for (const file of files) {
- if (!file.endsWith(`${path.sep}${KIMI_WIRE_FILE_NAME}`)) {
+ if (!file.endsWith(`/${KIMI_WIRE_FILE_NAME}`)) {
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.
| for (const file of files) { | |
| if (!file.endsWith(`${path.sep}${KIMI_WIRE_FILE_NAME}`)) { | |
| continue; | |
| } | |
| for (const file of files) { | |
| if (!file.endsWith(`/${KIMI_WIRE_FILE_NAME}`)) { | |
| continue; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/kimi/src/data-loader.ts` around lines 227 - 230, The endsWith check in
the for loop over files fails on Windows because tinyglobby returns POSIX-style
paths; update the condition that uses path.sep so it works cross-platform—either
compare using a forward slash like file.endsWith(`/${KIMI_WIRE_FILE_NAME}`) or,
more robustly, use path.basename(file) === KIMI_WIRE_FILE_NAME (referencing the
for (const file of files) loop and KIMI_WIRE_FILE_NAME) to locate the wire file
regardless of path separator.
| // ~40 chars = ~10 tokens at 4 chars per token | ||
| const thinkContent = 'This is a test thinking process for the model to reason through.'; | ||
| const expectedReasoningTokens = Math.ceil(thinkContent.length / 4); |
There was a problem hiding this comment.
Misleading comment: string length is 65 chars, not ~40.
The comment states "~40 chars" but thinkContent is 65 characters ('This is a test thinking process for the model to reason through.'.length === 65), yielding 17 estimated tokens (Math.ceil(65 / 4)), not 10.
📝 Proposed fix to correct the comment
it('extracts reasoning tokens from ContentPart think messages', async () => {
- // ~40 chars = ~10 tokens at 4 chars per token
+ // 65 chars = 17 tokens at 4 chars per token (Math.ceil(65 / 4))
const thinkContent = 'This is a test thinking process for the model to reason through.';📝 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.
| // ~40 chars = ~10 tokens at 4 chars per token | |
| const thinkContent = 'This is a test thinking process for the model to reason through.'; | |
| const expectedReasoningTokens = Math.ceil(thinkContent.length / 4); | |
| // 65 chars = 17 tokens at 4 chars per token (Math.ceil(65 / 4)) | |
| const thinkContent = 'This is a test thinking process for the model to reason through.'; | |
| const expectedReasoningTokens = Math.ceil(thinkContent.length / 4); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/kimi/src/data-loader.ts` around lines 649 - 651, Update the misleading
comment near the thinkContent and expectedReasoningTokens declarations: the
string in thinkContent is 65 characters, not "~40 chars", so correct the comment
to reflect the actual length (or remove the approximate char count) and clarify
token estimation (e.g., "65 chars ≈ 17 tokens at 4 chars/token"), keeping the
Math.ceil(thinkContent.length / 4) calculation unchanged.
1 participant
estimate)
Summary by CodeRabbit