Repository navigation
fix(ccusage): use streaming to handle large JSONL files - #706
Conversation
Fixes #460 by implementing line-by-line streaming for JSONL file processing instead of loading entire files into memory. This prevents RangeError: Invalid string length errors when processing large session files (>500MB). Changes: Add processJSONLFileByLine() helper using Node.js readline, Update getEarliestTimestamp() to use streaming, Update loadDailyUsageData() to use streaming, Update loadSessionData() to use streaming, Update loadSessionUsageById() to use streaming, Update loadSessionBlockData() to use streaming All existing tests pass with the new streaming implementation.
WalkthroughRefactors internal JSONL processing in Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Loader as Data Loader
participant Stream as processJSONLFileByLine
participant FS as File System
rect rgb(245,250,255)
Note over Loader,Stream: Stream-based per-line processing
App->>Loader: request load or scan
Loader->>Stream: createReadStream(file) + readline
Stream->>FS: open file
loop per line
FS-->>Stream: emits line N
Stream->>Loader: onLine(line N, index)
Loader->>Loader: parse → validate → dedupe → compute cost → aggregate
alt invalid / duplicate
Loader-->>Stream: return (skip processing)
end
end
FS-->>Stream: EOF
Stream->>Loader: close/complete
Loader-->>App: return aggregated result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (4)apps/ccusage/src/**/*.ts📄 CodeRabbit inference engine (apps/ccusage/CLAUDE.md)
Files:
apps/ccusage/**/*.ts📄 CodeRabbit inference engine (apps/ccusage/CLAUDE.md)
Files:
**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/data-loader.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (18)📓 Common learnings📚 Learning: 2025-09-18T17:43:09.255ZApplied to files:
📚 Learning: 2025-10-24T19:52:04.416ZApplied to files:
📚 Learning: 2025-09-18T17:43:09.255ZApplied to files:
📚 Learning: 2025-09-18T16:06:37.474ZApplied to files:
📚 Learning: 2025-09-18T17:43:09.255ZApplied to files:
📚 Learning: 2025-09-18T17:43:09.255ZApplied to files:
📚 Learning: 2025-09-17T18:29:15.764ZApplied to files:
📚 Learning: 2025-09-18T16:06:37.474ZApplied to files:
📚 Learning: 2025-09-18T16:06:37.474ZApplied to files:
📚 Learning: 2025-09-17T18:29:15.764ZApplied to files:
📚 Learning: 2025-09-18T17:43:09.255ZApplied to files:
📚 Learning: 2025-09-18T16:06:37.474ZApplied to files:
📚 Learning: 2025-09-18T17:43:09.255ZApplied to files:
📚 Learning: 2025-09-18T17:43:09.255ZApplied to files:
📚 Learning: 2025-09-18T16:07:16.293ZApplied to files:
📚 Learning: 2025-09-18T16:07:16.293ZApplied to files:
📚 Learning: 2025-09-18T16:07:16.293ZApplied to files:
🔇 Additional comments (7)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/ccusage/src/data-loader.ts(11 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/ccusage/src/**/*.ts
📄 CodeRabbit inference engine (apps/ccusage/CLAUDE.md)
apps/ccusage/src/**/*.ts: Write tests in-source usingif (import.meta.vitest != null)blocks instead of separate test files
Use Vitest globals (describe,it,expect) without imports in test blocks
In tests, use current Claude 4 models (sonnet-4, opus-4)
Usefs-fixturewithcreateFixture()to simulate Claude data in tests
Only export symbols that are actually used by other modules
Do not use console.log; use the logger utilities fromsrc/logger.tsinstead
Files:
apps/ccusage/src/data-loader.ts
apps/ccusage/**/*.ts
📄 CodeRabbit inference engine (apps/ccusage/CLAUDE.md)
apps/ccusage/**/*.ts: NEVER useawait import()dynamic imports anywhere (especially in tests)
Prefer@praha/byethrowResult type for error handling instead of try-catch
Use.tsextensions for local imports (e.g.,import { foo } from './utils.ts')
Files:
apps/ccusage/src/data-loader.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use tab indentation and double quotes (ESLint formatting)
Do not use console.log; only allow where explicitly disabled via eslint-disable
Always use Node.js path utilities for file paths for cross-platform compatibility
Use .ts extensions for local file imports (e.g., import { foo } from './utils.ts')
Prefer @praha/byethrow Result type over traditional try-catch for functional error handling
Use Result.try() to wrap operations that may throw (e.g., JSON parsing)
Use Result.isFailure() for checking errors instead of negating isSuccess()
Use early return on failures (e.g., if (Result.isFailure(r)) continue) instead of ternary patterns
For async operations, create a wrapper using Result.try() and call it
Keep traditional try-catch only for complex file I/O or legacy code that’s hard to refactor
Always use Result.isFailure() and Result.isSuccess() type guards for clarity
Variables use camelCase naming
Types use PascalCase naming
Constants can use UPPER_SNAKE_CASE
Only export constants, functions, and types that are actually used by other modules
Do not export internal/private constants that are only used within the same file
Before exporting a constant, verify it is referenced by other modules
Use Vitest globals (describe, it, expect) without imports in test blocks
Never use await import() dynamic imports anywhere in the codebase
Never use dynamic imports inside Vitest test blocks
Use fs-fixture createFixture() for mock Claude data directories in tests
All tests must use current Claude 4 models (not Claude 3)
Test coverage should include both Sonnet and Opus models
Model names in tests must exactly match LiteLLM pricing database entries
Use logger.ts instead of console.log for logging
Files:
apps/ccusage/src/data-loader.ts
**/data-loader.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Silently skip malformed JSONL lines during parsing in data-loader.ts
Files:
apps/ccusage/src/data-loader.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T17:43:09.255Z
Learning: Applies to **/data-loader.ts : Silently skip malformed JSONL lines during parsing in data-loader.ts
📚 Learning: 2025-09-18T17:43:09.255Z
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T17:43:09.255Z
Learning: Applies to **/data-loader.ts : Silently skip malformed JSONL lines during parsing in data-loader.ts
Applied to files:
apps/ccusage/src/data-loader.ts
🔇 Additional comments (6)
apps/ccusage/src/data-loader.ts (6)
21-21: LGTM! Appropriate streaming imports added.The imports of
createReadStreamandcreateInterfaceare correct for implementing streaming-based JSONL processing.Also applies to: 25-25
564-593: LGTM! Streaming implementation correctly preserves earliest timestamp logic.The function now uses streaming to scan the file for the earliest timestamp without loading the entire file into memory. The logic correctly compares all timestamps to find the earliest one.
781-818: LGTM! Streaming implementation preserves all logic correctly.The changes successfully migrate to streaming while:
- Extracting project name once per file (good optimization at line 783)
- Preserving deduplication logic with
processedHashes- Correctly using
returninstead ofcontinuein callbacks- Maintaining cost calculation behavior
950-987: LGTM! Session data loading correctly migrated to streaming.The streaming implementation preserves all session extraction and deduplication logic. Control flow changes from
continuetoreturnare appropriate for the callback context.
1121-1140: LGTM! Session usage by ID correctly uses streaming.The function properly streams through the session file, accumulating entries and costs without loading the entire file into memory.
1362-1406: LGTM! Session block data loading correctly migrated to streaming.The streaming implementation preserves deduplication and cost calculation logic. The debug logging at line 1404 appropriately uses
logger.debugper coding guidelines.
| /** | ||
| * Process a JSONL file line by line using streams to avoid memory issues with large files | ||
| * @param filePath - Path to the JSONL file | ||
| * @param processLine - Callback function to process each line | ||
| */ | ||
| async function processJSONLFileByLine( | ||
| filePath: string, | ||
| processLine: (line: string, lineNumber: number) => void | Promise<void>, | ||
| ): Promise<void> { | ||
| const fileStream = createReadStream(filePath, { encoding: 'utf-8' }); | ||
| const rl = createInterface({ | ||
| input: fileStream, | ||
| crlfDelay: Number.POSITIVE_INFINITY, | ||
| }); | ||
|
|
||
| let lineNumber = 0; | ||
| for await (const line of rl) { | ||
| lineNumber++; | ||
| if (line.trim().length === 0) { | ||
| continue; | ||
| } | ||
| await processLine(line, lineNumber); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add error handling for stream errors.
The function doesn't handle potential errors from the file stream or readline interface. While the for await loop handles cleanup, stream errors should be caught to prevent unhandled promise rejections.
Consider adding error handlers:
async function processJSONLFileByLine(
filePath: string,
processLine: (line: string, lineNumber: number) => void | Promise<void>,
): Promise<void> {
const fileStream = createReadStream(filePath, { encoding: 'utf-8' });
const rl = createInterface({
input: fileStream,
crlfDelay: Number.POSITIVE_INFINITY,
});
+ fileStream.on('error', (error) => {
+ logger.debug(`Error reading file ${filePath}: ${error instanceof Error ? error.message : String(error)}`);
+ throw error;
+ });
+
let lineNumber = 0;
for await (const line of rl) {
lineNumber++;
if (line.trim().length === 0) {
continue;
}
await processLine(line, lineNumber);
}
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Thank you for the review! However, I respectfully disagree with this suggestion. The current implementation is correct and follows Node.js best practices. Here's why:
for await...of already handles stream errors
Node.js's for await...of automatically detects and propagates stream errors. I verified this behavior with a test:
// Test: File not found error
const fileStream = createReadStream('/nonexistent/file.jsonl', { encoding: 'utf-8' });
const rl = createInterface({ input: fileStream });
for await (const line of rl) {
// ...
}
// → Throws: ENOENT: no such file or directoryThe error is automatically thrown and can be caught by the caller's try-catch block. Adding an explicit error handler provides no additional benefit.
All callers have proper error handling
Every function that calls processJSONLFileByLine() already handles errors appropriately:
export async function getEarliestTimestamp(filePath: string): Promise<Date | null> {
try {
await processJSONLFileByLine(filePath, (line) => { ... });
return earliestDate;
}
catch (error) {
logger.debug(`Failed to get earliest timestamp for ${filePath}:`, error);
return null;
}
}This pattern is used in all 5 callers: getEarliestTimestamp(), loadDailyUsageData(), loadSessionData(), loadSessionUsageById(), and loadSessionBlockData().
The suggested code has issues
fileStream.on('error', (error) => {
logger.debug(...);
throw error; // ← Problem: throwing inside an event handler
});Throwing inside an event handler occurs outside the async context and can lead to unhandled promise rejections. This pattern actually makes error handling worse, not better.
Node.js documentation
The Node.js readline documentation shows that when using for await...of, explicit error handlers are not needed. The async iterator handles cleanup and error propagation automatically.
TL;DR: The current implementation correctly relies on for await...of's built-in error handling, and all callers properly handle errors. Adding an explicit error handler would add unnecessary complexity and potentially introduce bugs.
There was a problem hiding this comment.
@mkusaka, you're absolutely right, and I appreciate the detailed explanation! Thank you for taking the time to clarify this.
You've correctly identified that:
-
for await...ofhandles stream errors automatically - The async iterator protocol in Node.js ensures that stream errors are properly caught and propagated to the calling context, where they can be handled with standard try-catch blocks. -
My suggested pattern was flawed - You're right that throwing inside an event handler doesn't integrate properly with async/await and can lead to unhandled promise rejections. This would indeed make error handling worse, not better.
-
The current implementation follows Node.js best practices - Using
for await...ofwith readline is the recommended pattern in the Node.js documentation, and explicit error handlers on the stream are unnecessary.
Your implementation is correct as-is. The for await...of pattern provides clean, automatic error propagation, and all your callers properly handle errors at the appropriate level (either catching them explicitly like in getEarliestTimestamp(), or letting them propagate to higher-level error handlers).
I'll withdraw my original suggestion. Thank you for the correction!
✏️ Learnings added
Learnt from: mkusaka
PR: ryoppippi/ccusage#706
File: apps/ccusage/src/data-loader.ts:534-557
Timestamp: 2025-10-24T19:52:04.393Z
Learning: In Node.js, when using `for await...of` with readline interface and createReadStream, explicit error handlers on the stream are unnecessary. The async iterator protocol automatically propagates stream errors to the calling context where they can be caught with try-catch blocks. This is the recommended pattern per Node.js documentation and adding explicit stream error handlers can actually introduce bugs by throwing outside the async context.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T17:43:09.255Z
Learning: Applies to **/data-loader.ts : Silently skip malformed JSONL lines during parsing in data-loader.ts
commit: |
|
@mkusaka thanks! could you write a test for your function |
Add 7 test cases for the streaming JSONL processing function: - Basic line processing with correct line numbers - Empty line skipping - Async callback handling - Non-existent file error handling - Empty file handling - Files with only empty lines - Large file (600MB+) processing without RangeError The 600MB test validates that streaming can handle files exceeding Node.js string length limit (~512MB) which caused the original issue.
…saka/ccusage into fix/stream-large-jsonl-files
|
Thanks for the review! I've added tests for processJSONLFileByLine. It covers 7 test cases: Basic functionality:
Performance/boundary test:
Commit: 556d10d |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ccusage/src/data-loader.ts (1)
1496-1540: Silently skip malformed JSON lines; remove per‑line debug logging.Per prior decision, malformed lines in data-loader should be skipped without per‑line logs to avoid noisy output on large files.
Apply this diff:
- await processJSONLFileByLine(file, async (line) => { - try { - const parsed = JSON.parse(line) as unknown; - const result = v.safeParse(usageDataSchema, parsed); - if (!result.success) { - return; - } - const data = result.output; + await processJSONLFileByLine(file, async (line) => { + const parsedR = Result.try(() => JSON.parse(line) as unknown); + if (Result.isFailure(parsedR)) return; + const result = v.safeParse(usageDataSchema, parsedR.value); + if (!result.success) return; + const data = result.output; ... - } - catch (error) { - // Skip invalid JSON lines but log for debugging purposes - logger.debug(`Skipping invalid JSON line in 5-hour blocks: ${error instanceof Error ? error.message : String(error)}`); - } + } });Based on learnings
🧹 Nitpick comments (5)
apps/ccusage/src/data-loader.ts (5)
642-689: Make the large‑file test CI‑friendly.Writing/reading ~600MB risks slow/flaky CI. Gate size in CI and extend timeout.
Apply this diff:
- it('should process large files (600MB+) without RangeError', async () => { + it('should process large files without RangeError', async () => { ... - // Target 600MB file (this would cause RangeError with readFile in Node.js) - const targetMB = 600; + // Keep CI light; still exercises streaming path + const targetMB = process.env.CI ? 64 : 600;Optionally bump timeout if needed:
- it('should process large files without RangeError', async () => { + it('should process large files without RangeError', async () => { /* ... */ }, 120000)Please confirm CI runtime stays acceptable after this change.
702-717: Prefer Result.try() over try/catch for JSON parsing.Aligns with project error‑handling style and keeps control flow purely early‑return.
Apply this diff:
- await processJSONLFileByLine(filePath, (line) => { - try { - const json = JSON.parse(line) as Record<string, unknown>; - if (json.timestamp != null && typeof json.timestamp === 'string') { - const date = new Date(json.timestamp); - if (!Number.isNaN(date.getTime())) { - if (earliestDate == null || date < earliestDate) { - earliestDate = date; - } - } - } - } - catch { - // Skip invalid JSON lines - } - }); + await processJSONLFileByLine(filePath, (line) => { + const parsed = Result.try(() => JSON.parse(line) as Record<string, unknown>); + if (Result.isFailure(parsed)) return; // skip malformed + const json = parsed.value; + const ts = (json as Record<string, unknown>).timestamp; + if (typeof ts === "string") { + const date = new Date(ts); + if (!Number.isNaN(date.getTime()) && (earliestDate == null || date < earliestDate)) { + earliestDate = date; + } + } + });As per coding guidelines
919-951: Streaming parse callback: switch to Result.try() and early returns.Removes try/catch noise and follows the repository pattern.
Apply this diff:
- await processJSONLFileByLine(file, async (line) => { - try { - const parsed = JSON.parse(line) as unknown; - const result = v.safeParse(usageDataSchema, parsed); - if (!result.success) { - return; - } - const data = result.output; + await processJSONLFileByLine(file, async (line) => { + const parsedR = Result.try(() => JSON.parse(line) as unknown); + if (Result.isFailure(parsedR)) return; // malformed JSONL + const result = v.safeParse(usageDataSchema, parsedR.value); + if (!result.success) return; // schema mismatch + const data = result.output; ... - if (isDuplicateEntry(uniqueHash, processedHashes)) { - // Skip duplicate message - return; - } + if (isDuplicateEntry(uniqueHash, processedHashes)) return; // duplicate ... - } - catch { - // Skip invalid JSON lines - } - }); + });As per coding guidelines
1084-1121: Same refactor here: use Result.try() and early returns.Apply this diff:
- await processJSONLFileByLine(file, async (line) => { - try { - const parsed = JSON.parse(line) as unknown; - const result = v.safeParse(usageDataSchema, parsed); - if (!result.success) { - return; - } - const data = result.output; + await processJSONLFileByLine(file, async (line) => { + const parsedR = Result.try(() => JSON.parse(line) as unknown); + if (Result.isFailure(parsedR)) return; + const result = v.safeParse(usageDataSchema, parsedR.value); + if (!result.success) return; + const data = result.output; ... - if (isDuplicateEntry(uniqueHash, processedHashes)) { - // Skip duplicate message - return; - } + if (isDuplicateEntry(uniqueHash, processedHashes)) return; ... - } - catch { - // Skip invalid JSON lines - } - }); + });As per coding guidelines
1255-1274: Use Result.try() in loadSessionUsageById as well.Apply this diff:
- await processJSONLFileByLine(file, async (line) => { - try { - const parsed = JSON.parse(line) as unknown; - const result = v.safeParse(usageDataSchema, parsed); - if (!result.success) { - return; - } - const data = result.output; + await processJSONLFileByLine(file, async (line) => { + const parsedR = Result.try(() => JSON.parse(line) as unknown); + if (Result.isFailure(parsedR)) return; + const result = v.safeParse(usageDataSchema, parsedR.value); + if (!result.success) return; + const data = result.output; ... - } - catch { - // Skip invalid JSON lines - } - }); + });As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/ccusage/src/data-loader.ts(11 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/ccusage/src/**/*.ts
📄 CodeRabbit inference engine (apps/ccusage/CLAUDE.md)
apps/ccusage/src/**/*.ts: Write tests in-source usingif (import.meta.vitest != null)blocks instead of separate test files
Use Vitest globals (describe,it,expect) without imports in test blocks
In tests, use current Claude 4 models (sonnet-4, opus-4)
Usefs-fixturewithcreateFixture()to simulate Claude data in tests
Only export symbols that are actually used by other modules
Do not use console.log; use the logger utilities fromsrc/logger.tsinstead
Files:
apps/ccusage/src/data-loader.ts
apps/ccusage/**/*.ts
📄 CodeRabbit inference engine (apps/ccusage/CLAUDE.md)
apps/ccusage/**/*.ts: NEVER useawait import()dynamic imports anywhere (especially in tests)
Prefer@praha/byethrowResult type for error handling instead of try-catch
Use.tsextensions for local imports (e.g.,import { foo } from './utils.ts')
Files:
apps/ccusage/src/data-loader.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use tab indentation and double quotes (ESLint formatting)
Do not use console.log; only allow where explicitly disabled via eslint-disable
Always use Node.js path utilities for file paths for cross-platform compatibility
Use .ts extensions for local file imports (e.g., import { foo } from './utils.ts')
Prefer @praha/byethrow Result type over traditional try-catch for functional error handling
Use Result.try() to wrap operations that may throw (e.g., JSON parsing)
Use Result.isFailure() for checking errors instead of negating isSuccess()
Use early return on failures (e.g., if (Result.isFailure(r)) continue) instead of ternary patterns
For async operations, create a wrapper using Result.try() and call it
Keep traditional try-catch only for complex file I/O or legacy code that’s hard to refactor
Always use Result.isFailure() and Result.isSuccess() type guards for clarity
Variables use camelCase naming
Types use PascalCase naming
Constants can use UPPER_SNAKE_CASE
Only export constants, functions, and types that are actually used by other modules
Do not export internal/private constants that are only used within the same file
Before exporting a constant, verify it is referenced by other modules
Use Vitest globals (describe, it, expect) without imports in test blocks
Never use await import() dynamic imports anywhere in the codebase
Never use dynamic imports inside Vitest test blocks
Use fs-fixture createFixture() for mock Claude data directories in tests
All tests must use current Claude 4 models (not Claude 3)
Test coverage should include both Sonnet and Opus models
Model names in tests must exactly match LiteLLM pricing database entries
Use logger.ts instead of console.log for logging
Files:
apps/ccusage/src/data-loader.ts
**/data-loader.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Silently skip malformed JSONL lines during parsing in data-loader.ts
Files:
apps/ccusage/src/data-loader.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T17:43:09.255Z
Learning: Applies to **/data-loader.ts : Silently skip malformed JSONL lines during parsing in data-loader.ts
📚 Learning: 2025-09-18T17:43:09.255Z
Learnt from: CR
PR: ryoppippi/ccusage#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T17:43:09.255Z
Learning: Applies to **/data-loader.ts : Silently skip malformed JSONL lines during parsing in data-loader.ts
Applied to files:
apps/ccusage/src/data-loader.ts
📚 Learning: 2025-10-24T19:52:04.416Z
Learnt from: mkusaka
PR: ryoppippi/ccusage#706
File: apps/ccusage/src/data-loader.ts:534-557
Timestamp: 2025-10-24T19:52:04.416Z
Learning: In Node.js, when using `for await...of` with readline interface and createReadStream, explicit error handlers on the stream are unnecessary. The async iterator protocol automatically propagates stream errors to the calling context where they can be caught with try-catch blocks. This is the recommended pattern per Node.js documentation and adding explicit stream error handlers can actually introduce bugs by throwing outside the async context.
Applied to files:
apps/ccusage/src/data-loader.ts
🔇 Additional comments (1)
apps/ccusage/src/data-loader.ts (1)
534-557: Streaming helper is correct; error handling is appropriate with for‑await‑of.Implementation is lean, backpressure‑friendly, and lets iterator propagate stream errors to callers. Good choice.
Based on learnings
Replace await import('node:fs/promises') with createWriteStream for building
large test files. This adheres to the project rule against dynamic imports
and provides better memory efficiency through proper backpressure handling.
Addresses CodeRabbit review feedback.
|
Thanks.we need to move the test in the existing test block but overall lgtm |
Fix TypeScript error by wrapping resolve calls in arrow functions and adding explicit Promise<void> type annotations. This ensures type safety for event listener callbacks.
Move processJSONLFileByLine tests from standalone block into the main test block with other data-loader tests, as requested in code review.
2 participants
Summary
Fixes #460, fixes #241, fixes #703 by implementing line-by-line streaming for JSONL file processing instead of loading entire files into memory. This prevents
RangeError: Invalid string lengtherrors when processing large session files (>500MB).Problem
Users reported crashes with
RangeError: Invalid string lengthwhen ccusage attempted to read very large session files (585MB+). The previous implementation loaded entire files into memory usingreadFile(), which hits Node.js's string length limit (~512MB).Affected Issues:
Solution
Implemented streaming-based JSONL processing using Node.js
readlineinterface to handle files of any size without memory issues.Key Changes:
processJSONLFileByLine()helper function usingcreateReadStreamandreadlineUpdated Functions:
getEarliestTimestamp()loadDailyUsageData()loadSessionData()loadSessionUsageById()loadSessionBlockData()Testing
Small file test (526 lines) - Perfect match with old approach
Large file test (1.1GB, 892,920 lines) - Processed in 2.21s (~403K lines/sec)
All 256 existing tests pass - No behavioral changes
Summary by CodeRabbit