Repository navigation
fix console log reassembler eating multi-chunk lines - #56
Conversation
add() returning false meant two different things: a real gap, or just the first chunk into a fresh/flushed instance. a caller following the doc comment (flush after every false) would nuke _lastIndex on every chunk and never actually reassemble a straddled log line. added hasCompletedRun so callers can tell the difference, and pointed the docs at it instead of add()'s raw return. add()/flush() behavior unchanged.
There was a problem hiding this comment.
Sorry @abdulsaheel, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 6 days and 18 hours by commenting @sourcery-ai review. Upgrade to get a review now.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughChangesConsole log reassembly
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The explicit completion state and regression test support the intended console log reassembly behavior, so the change is mergeable. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Reviewer's GuideThe PR fixes an ambiguous reassembler API contract by adding Sequence diagram for safe console log chunk reassemblysequenceDiagram
participant Caller
participant Reassembler as ConsoleLogReassembler
Caller->>Reassembler: add(chunk1)
Reassembler-->>Caller: false
Caller->>Reassembler: hasCompletedRun
Reassembler-->>Caller: false
Caller->>Reassembler: add(chunk2)
Reassembler-->>Caller: true
Caller->>Reassembler: add(chunk3)
Reassembler-->>Caller: true
Caller->>Reassembler: add(chunk4)
Reassembler-->>Caller: false
Caller->>Reassembler: hasCompletedRun
Reassembler-->>Caller: true
Caller->>Reassembler: flush()
Reassembler-->>Caller: completed run
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
add() returning false was ambiguous - could mean a real gap, or just the first chunk into a fresh/flushed instance. the doc comment told callers to flush() on any false, but flush()'s no-completed branch always resets _lastIndex, so a caller following that advice would flush after every single chunk and never actually reassemble anything longer than one chunk.
not wired into edge yet so nothing shipped is broken, but it'd bite the first real caller.
fix: added hasCompletedRun getter, pointed the docs at checking that instead of add()'s return value. didn't touch add()/flush() logic or signatures so the existing gap test still passes untouched. added a test that reproduces the 4-chunk-line-split-into-4-fragments scenario and proves it's fixed.
Summary by Sourcery
Fix console log reassembly so contiguous multi-chunk lines remain intact across fresh and flushed instances.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
Bug Fixes
Tests