Conversation
Redirect parse_events to delegate to _parse_events_from_offset with include_partial_tail=True, aligning tests with the production code path. Add include_partial_tail keyword arg to _parse_events_from_offset (matching the vscode_parser pattern). Update architecture.md step 2 to describe the actual pipeline. Add regression test verifying parse_events and get_cached_events agree on partial-tail files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR aligns the public parse_events() API with the production parsing path by delegating to the binary-mode _parse_events_from_offset() implementation, reducing behavioral drift between tests and real-world usage.
Changes:
- Added
include_partial_tailto_parse_events_from_offset()to control how malformed, non-newline-terminated tail lines are handled. - Rewrote
parse_events()to delegate to_parse_events_from_offset(..., include_partial_tail=True). - Updated architecture docs and adjusted/added tests to validate offset-based warnings and partial-tail parity with the cache layer.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/copilot_usage/parser.py |
Adds include_partial_tail and redirects parse_events() to the canonical binary-mode parser. |
src/copilot_usage/docs/architecture.md |
Updates pipeline docs to describe the actual parsing/caching implementation. |
tests/copilot_usage/test_parser.py |
Updates warning expectations to byte offsets, adjusts invalid-UTF-8 behavior tests, and adds a partial-tail parity regression test. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Commit pushed:
|
Contributor
|
Commit pushed:
|
Rename test methods with stale UnicodeDecodeError names to reflect actual binary-mode behavior (invalid UTF-8 bytes cause JSON skip): - test_unicode_decode_error_returns_partial → test_invalid_utf8_line_skipped_valid_event_returned - test_unicode_decode_error_full_file → test_entirely_invalid_utf8_returns_empty_list Update docstrings and inline comments accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
microsasa
added
the
aw-review-response-attempted
- Strengthen test_invalid_utf8_line_skipped_valid_event_returned to assert exactly 1 event instead of allowing 0 (binary mode is deterministic) - Document UnicodeDecodeError partial-return behavior in parse_events docstring Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Commit pushed:
|
microsasa
added
aw-review-response-1
Add test for malformed partial tail with include_partial_tail=True to regression-protect the flag's warn-and-skip behavior when the final non-newline-terminated line fails JSON decoding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Commit pushed:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Quality Gate: APPROVE (Medium impact — auto-merge eligible)
Evaluated across code quality and blast radius:
- Refactoring:
parse_eventsnow delegates to_parse_events_from_offset(the production binary-mode path), eliminating a divergent text-mode code path that was only exercised in tests. - Minimal impl change: Single condition addition (
and not include_partial_tail) with clear docstring. - Tests: Regression test for partial-tail parity between
parse_eventsandget_cached_events, malformed-tail coverage, byte-offset assertion updates — all meaningful. - Docs: Architecture doc updated to reflect actual implementation.
- CI: All 9 checks green. All 8 review threads resolved.
- No API contract changes, no new dependencies, no security-sensitive code.
microsasa
deleted the
fix/987-parse-events-delegates-to-offset-4cf92f847d6533d5
branch
This was referenced Apr 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #987
Changes
parse_eventsnow delegates to_parse_events_from_offset— the same binary-mode implementation used by the production cache layer (get_cached_events). This eliminates the divergence where tests exercised a text-mode code path that was never used in production.src/copilot_usage/parser.pyinclude_partial_tailkeyword argument to_parse_events_from_offset(mirrors the existing pattern invscode_parser.py). WhenTrue, a final non-newline-terminated line is included rather than triggering an early break.parse_eventsbody to delegate:events, _ = _parse_events_from_offset(events_path, 0, include_partial_tail=True)src/copilot_usage/docs/architecture.md_parse_events_from_offsetaccessed viaget_cached_events, with caching and incremental parsing.tests/copilot_usage/test_parser.pyTestParseEventsPartialTailParity): writes a file whose last line is valid JSON without a trailing newline, asserts thatparse_eventsandget_cached_eventsreturn the same events.UnicodeDecodeErrorcrash — strictly better recovery).All existing tests pass.
make checkclean (lint, typecheck, security, 99% coverage).Warning
The following domain was blocked by the firewall during workflow execution:
astral.shTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.