Restore deriveSegmentText() to fix missing matchedText - #155
Merged
shouze merged 1 commit intoAug 23, 2026
Conversation
shouze
changed the title
shouze
force-pushed
the
fix/derive-segment-text
branch
from
August 23, 2026 13:58
ec46820 to
5f83337
Compare
|
Coverage after merging fix/derive-segment-text into fix/regex-local-filter-full-content will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
1 similar comment
|
Coverage after merging fix/derive-segment-text into fix/regex-local-filter-full-content will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restores deriveSegmentText() in src/api.ts so text-match segments missing the text field from GitHub’s API derive their matched text from fragment.slice(indices[0], indices[1]), preventing empty/missing matched text in downstream output.
Changes:
- Reintroduced
deriveSegmentText(fragment, seg)and applied it duringfetchAllResults()text-match normalization. - Added unit tests for
deriveSegmentText()covering provided text, omitted text, and invalid indices. - Added a
fetchAllResults()regression test ensuring derived segment text is produced whensegment.textis omitted.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/api.ts | Restores deriveSegmentText() and uses it when mapping raw GitHub text-match segments into normalized matches. |
| src/api.test.ts | Adds unit coverage for deriveSegmentText() and an integration-style regression test for fetchAllResults() deriving missing segment text. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+170
to
+174
| it("derives matchedText from fragment + indices when GitHub omits the segment's text field (issue #151)", async () => { | ||
| const fakeItem = { | ||
| path: "src/foo.ts", | ||
| html_url: "https://github.com/org/repo/blob/main/src/foo.ts", | ||
| repository: { full_name: "org/repo", archived: false }, |
shouze
force-pushed
the
fix/derive-segment-text
branch
from
August 23, 2026 14:26
5f83337 to
7879ae5
Compare
|
Coverage after merging fix/derive-segment-text into fix/regex-local-filter-full-content will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
1 similar comment
|
Coverage after merging fix/derive-segment-text into fix/regex-local-filter-full-content will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
shouze
force-pushed
the
fix/derive-segment-text
branch
from
August 23, 2026 14:36
7879ae5 to
5d605b1
Compare
|
Coverage after merging fix/derive-segment-text into fix/regex-local-filter-full-content will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
1 similar comment
|
Coverage after merging fix/derive-segment-text into fix/regex-local-filter-full-content will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
PR #133 introduced deriveSegmentText() to derive a text-match segment's matched text from fragment.slice(indices[0], indices[1]) when GitHub omits the text field, but the current code on this branch still defaulted to an empty string in that case. Reintroduced the helper and wired it into fetchAllResults()'s mapping, with unit tests for the helper itself and an integration test through fetchAllResults() covering the omitted-text case. Closes #151
shouze
force-pushed
the
fix/derive-segment-text
branch
from
August 23, 2026 14:51
5d605b1 to
e4b7220
Compare
|
Coverage after merging fix/derive-segment-text into fix/regex-local-filter-full-content will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
1 similar comment
|
Coverage after merging fix/derive-segment-text into fix/regex-local-filter-full-content will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
What does this PR do?
Closes #151.
Pull request #133 introduced
deriveSegmentText()to derive a text-match segment's matched text fromfragment.slice(indices[0], indices[1])when GitHub omits thetextfield, but the current code on this branch still defaulted to an empty string in that case. This reintroduces the helper insrc/api.tsand wires it intofetchAllResults()'s mapping.How did you verify your code works?
deriveSegmentText()itself (provided text, omitted text, out-of-range indices, inverted indices).fetchAllResults()covering atext_matchessegment with notextfield, asserting the derived value is correct.