Skip to content

fix: Handle missing segments from ImpactedFileComparison response #3961

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
calvin-codecov wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: Handle missing segments from ImpactedFileComparison response #3961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14b3211
fix: Handle missing segments from ImpactedFileComparison response
calvin-codecov Jan 12, 2026
3eb024f
Merge branch 'main' into cy/file_diff_400
calvin-codecov Jan 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions ...DetailPage/CommitCoverage/routes/FilesChangedTab/shared/CommitFileDiff/CommitFileDiff.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,20 @@ import {
} from 'ui/VirtualRenderers/VirtualDiffRenderer'

function transformSegmentsToLineData(
segments: ImpactedFileType['segments']['results'] | undefined,
segments: ImpactedFileType['segments'] | undefined,
ignoredUploadIds: number[]
) {
if (!segments) {
if (
!segments ||
segments.__typename !== 'SegmentComparisons' ||
!segments.results
) {
return []
}

const ignoredUploadIdsSet = new Set(ignoredUploadIds)

return segments.map((segment) => {
return segments.results.map((segment) => {
// we need to create a string of the diff content for the virtual diff renderer text area
let newDiffContent = ''
const lineData: LineData[] = []
Expand Down
10 changes: 7 additions & 3 deletions ...Coverage/routes/IndirectChangesTab/IndirectChangesTable/CommitFileDiff/CommitFileDiff.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,20 @@ import {
} from 'ui/VirtualRenderers/VirtualDiffRenderer'

function transformSegmentsToLineData(
segments: ImpactedFileType['segments']['results'] | undefined,
segments: ImpactedFileType['segments'] | undefined,
ignoredUploadIds: number[]
) {
if (!segments) {
if (
!segments ||
segments.__typename !== 'SegmentComparisons' ||
!segments.results
) {
return []
}

const ignoredUploadIdsSet = new Set(ignoredUploadIds)

return segments.map((segment) => {
return segments.results.map((segment) => {
// we need to create a string of the diff content for the virtual diff renderer text area
let newDiffContent = ''
const lineData: LineData[] = []
Expand Down
71 changes: 61 additions & 10 deletions ...equestPage/PullCoverage/routes/FilesChangedTab/FilesChanged/PullFileDiff/PullFileDiff.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ import { Fragment, useMemo } from 'react'
import { useLocation, useParams } from 'react-router-dom'

import { useNavLinks } from 'services/navigation/useNavLinks'
import {
type PullImpactedFile,
useSingularImpactedFileComparison,
} from 'services/pull/useSingularImpactedFileComparison'
import { useSingularImpactedFileComparison } from 'services/pull/useSingularImpactedFileComparison'
import { transformImpactedPullFileToDiff } from 'services/pull/utils'
import { useRepoOverview } from 'services/repo'
import A from 'ui/A'
import CodeRendererInfoRow from 'ui/CodeRenderer/CodeRendererInfoRow'
Expand All @@ -14,13 +12,21 @@ import { CoverageValue } from 'ui/VirtualRenderers/types'
import { LineData } from 'ui/VirtualRenderers/VirtualDiffRenderer'

function transformSegmentsToLineData(
segments: PullImpactedFile['segments']['results'] | undefined
segments:
| NonNullable<
ReturnType<typeof transformImpactedPullFileToDiff>
>['segments']
| undefined
) {
if (!segments) {
if (
!segments ||
segments.__typename !== 'SegmentComparisons' ||
!segments.results
) {
return []
}

return segments.map((segment) => {
return segments.results.map((segment) => {
// we need to create a string of the diff content for the virtual diff renderer text area
let newDiffContent = ''
const lineData: LineData[] = []
Expand Down Expand Up @@ -57,7 +63,7 @@ function DiffRenderer({
impactedFile,
path,
}: {
impactedFile: ReturnType<typeof useSingularImpactedFileComparison>['data']
impactedFile: ReturnType<typeof transformImpactedPullFileToDiff>
path: string
}) {
const { pullFileView } = useNavLinks()
Expand Down Expand Up @@ -134,13 +140,44 @@ function ErrorDisplayMessage() {
)
}

function UnknownPathErrorDisplayMessage({ path }: { path: string }) {
const location = useLocation()

return (
<p className="border border-solid border-ds-gray-tertiary p-4">
There was a problem getting the source code from your provider by path
for: <strong>{path}</strong>. Unable to show line by line coverage.
<br />
<span>
If you continue to experience this issue, please try{' '}
<A
to={{ pageName: 'login', options: { to: location.pathname } }}
hook={undefined}
isExternal={undefined}
>
logging in
</A>{' '}
again to refresh your credentials. Otherwise, please visit our{' '}
<A
to={{ pageName: 'pathFixing', options: { to: location.pathname } }}
hook={undefined}
isExternal={undefined}
>
Path Fixing
</A>{' '}
documentation for troubleshooting tips.
</span>
</p>
)
}
Copy link

@cursor cursor bot Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Identical component duplicated across multiple files

Low Severity

The UnknownPathErrorDisplayMessage component is defined identically in two separate PullFileDiff.tsx files in this PR. The same component also already exists in the CommitFileDiff files, resulting in four total copies of identical code. This increases maintenance burden and risks inconsistent bug fixes if the error message or styling needs to change. While this follows an existing pattern in the codebase (similar to ErrorDisplayMessage), extracting it to a shared location would reduce duplication.

Additional Locations (1)

Fix in Cursor Fix in Web


interface PullFileDiffProps {
path: string | null | undefined
}

function PullFileDiff({ path }: PullFileDiffProps) {
const { provider, owner, repo, pullId } = useParams<URLParams>()
const { data } = useSingularImpactedFileComparison({
const { data: impactedFile } = useSingularImpactedFileComparison({
provider,
owner,
repo,
Expand All @@ -149,10 +186,24 @@ function PullFileDiff({ path }: PullFileDiffProps) {
filters: { hasUnintendedChanges: false },
})

if (!data || typeof path !== 'string') {
const segments = impactedFile?.segments

if (
!impactedFile ||
typeof path !== 'string' ||
!segments ||
segments.__typename === 'ProviderError'
) {
return <ErrorDisplayMessage />
}

if (segments.__typename === 'UnknownPath') {
return <UnknownPathErrorDisplayMessage path={path} />
}

// Transform the data to form needed for rendering
const data = transformImpactedPullFileToDiff(impactedFile)

return <DiffRenderer impactedFile={data} path={path} />
}

Expand Down
71 changes: 61 additions & 10 deletions ...ages/PullRequestPage/PullCoverage/routes/IndirectChangesTab/PullFileDiff/PullFileDiff.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ import { Fragment, useMemo } from 'react'
import { useLocation, useParams } from 'react-router-dom'

import { useNavLinks } from 'services/navigation/useNavLinks'
import {
PullImpactedFile,
useSingularImpactedFileComparison,
} from 'services/pull/useSingularImpactedFileComparison'
import { useSingularImpactedFileComparison } from 'services/pull/useSingularImpactedFileComparison'
import { transformImpactedPullFileToDiff } from 'services/pull/utils'
import { useRepoOverview } from 'services/repo'
import A from 'ui/A'
import CodeRendererInfoRow from 'ui/CodeRenderer/CodeRendererInfoRow'
Expand All @@ -16,12 +14,20 @@ import {
} from 'ui/VirtualRenderers/VirtualDiffRenderer'

function transformSegmentsToLineData(
segments: PullImpactedFile['segments']['results'] | undefined
segments:
| NonNullable<
ReturnType<typeof transformImpactedPullFileToDiff>
>['segments']
| undefined
) {
if (!segments) {
if (
!segments ||
segments.__typename !== 'SegmentComparisons' ||
!segments.results
) {
return []
}
return segments.map((segment) => {
return segments.results.map((segment) => {
// we need to create a string of the diff content for the virtual diff renderer text area
let newDiffContent = ''
const lineData: LineData[] = []
Expand Down Expand Up @@ -54,7 +60,7 @@ function DiffRenderer({
impactedFile,
path,
}: {
impactedFile: ReturnType<typeof useSingularImpactedFileComparison>['data']
impactedFile: ReturnType<typeof transformImpactedPullFileToDiff>
path: string
}) {
const { pullFileView } = useNavLinks()
Expand Down Expand Up @@ -128,13 +134,44 @@ function ErrorDisplayMessage() {
)
}

function UnknownPathErrorDisplayMessage({ path }: { path: string }) {
const location = useLocation()

return (
<p className="border border-solid border-ds-gray-tertiary p-4">
There was a problem getting the source code from your provider by path
for: <strong>{path}</strong>. Unable to show line by line coverage.
<br />
<span>
If you continue to experience this issue, please try{' '}
<A
to={{ pageName: 'login', options: { to: location.pathname } }}
hook={undefined}
isExternal={undefined}
>
logging in
</A>{' '}
again to refresh your credentials. Otherwise, please visit our{' '}
<A
to={{ pageName: 'pathFixing', options: { to: location.pathname } }}
hook={undefined}
isExternal={undefined}
>
Path Fixing
</A>{' '}
documentation for troubleshooting tips.
</span>
</p>
)
}

interface PullFileDiffProps {
path: string | null | undefined
}

function PullFileDiff({ path }: PullFileDiffProps) {
const { provider, owner, repo, pullId } = useParams<URLParams>()
const { data } = useSingularImpactedFileComparison({
const { data: impactedFile } = useSingularImpactedFileComparison({
provider,
owner,
repo,
Expand All @@ -143,10 +180,24 @@ function PullFileDiff({ path }: PullFileDiffProps) {
filters: { hasUnintendedChanges: true },
})

if (!data || typeof path !== 'string') {
const segments = impactedFile?.segments

if (
!impactedFile ||
typeof path !== 'string' ||
!segments ||
segments.__typename === 'ProviderError'
) {
return <ErrorDisplayMessage />
}

if (segments.__typename === 'UnknownPath') {
return <UnknownPathErrorDisplayMessage path={path} />
}

// Transform the data to form needed for rendering
const data = transformImpactedPullFileToDiff(impactedFile)

return <DiffRenderer impactedFile={data} path={path} />
}

Expand Down
2 changes: 1 addition & 1 deletion src/services/comparison/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function transformImpactedFileToDiff(impactedFile: ImpactedFileType) {
return {
fileLabel,
headName: impactedFile?.headName,
segments: impactedFile?.segments?.results,
segments: impactedFile?.segments,
...(!!hashedPath && { hashedPath }),
}
}
49 changes: 32 additions & 17 deletions src/services/pull/fragments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,33 @@ fragment FileComparisonWithBase on Pull {

const CoverageLineSchema = z.enum(['H', 'M', 'P'])

const SegmentComparisonsSchema = z.object({
__typename: z.literal('SegmentComparisons'),
results: z.array(
z.object({
header: z.string(),
hasUnintendedChanges: z.boolean(),
lines: z.array(
z.object({
baseNumber: z.string().nullable(),
headNumber: z.string().nullable(),
baseCoverage: CoverageLineSchema.nullable(),
headCoverage: CoverageLineSchema.nullable(),
content: z.string().nullable(),
})
),
})
),
})

const UnknownPathSchema = z.object({
__typename: z.literal('UnknownPath'),
})

const ProviderErrorSchema = z.object({
__typename: z.literal('ProviderError'),
})

export const ImpactedFileSchema = z.object({
headName: z.string().nullable(),
hashedPath: z.string(),
Expand All @@ -276,23 +303,11 @@ export const ImpactedFileSchema = z.object({
percentCovered: z.number().nullable(),
})
.nullable(),
segments: z.object({
results: z.array(
z.object({
header: z.string(),
hasUnintendedChanges: z.boolean(),
lines: z.array(
z.object({
baseNumber: z.string().nullable(),
headNumber: z.string().nullable(),
baseCoverage: CoverageLineSchema.nullable(),
headCoverage: CoverageLineSchema.nullable(),
content: z.string().nullable(),
})
),
})
),
}),
segments: z.discriminatedUnion('__typename', [
SegmentComparisonsSchema,
UnknownPathSchema,
ProviderErrorSchema,
]),
})

export const ComparisonSchema = z.object({
Expand Down
Loading
Loading