Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Add direct data-manager boundary coverage and tighten the mock expectation to verify the lookup position.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Fixes premature reverse EOS by recovering predecessors from the TSB store when weak links are unavailable.
Changes:
- Adds position-based predecessor recovery.
- Extends data-manager APIs and test doubles.
- Adds reverse traversal and EOS regression tests.
| File | Summary |
|---|---|
test/utests/tests/AampTsbReader/FunctionalTests.cpp |
Adds reverse recovery and EOS tests. |
test/utests/mocks/MockTSBDataManager.h |
Adds the predecessor lookup mock. |
test/utests/fakes/FakeTsbDataManager.cpp |
Delegates predecessor lookup to the mock. |
AampTsbReader.h |
Declares predecessor recovery support. |
AampTsbReader.cpp |
Recovers predecessors and updates reverse EOS handling. |
AampTsbDataManager.h |
Exposes predecessor lookup. |
AampTsbDataManager.cpp |
Implements strict predecessor lookup. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
The TSB reader raised end-of-stream whenever a fragment's weak prev link resolved to null, which also happens when the predecessor object is released while earlier content still exists — causing rewind to jump to the start of the TSB.
Make reverse EOS position-authoritative: only top at the genuine first fragment, otherwise recover the predecessor from the store via GetFragmentBefore().