Skip to content

Refactor source report to use sparse line metrics#6153

Merged
yomete merged 5 commits into
mainfrom
source-report
Jan 15, 2026
Merged

Refactor source report to use sparse line metrics#6153
yomete merged 5 commits into
mainfrom
source-report

Conversation

@yomete
Copy link
Copy Markdown
Contributor

@yomete yomete commented Jan 15, 2026

The source report will no longer return an error if the source file is not present and we also added explicit line numbers to the Arrow schema that is returned. Now, only lines with actual profiling data are included in the response, rather than returning an array with one entry per line in the source file.

Changes:
Backend returns empty source field instead of error when source file is missing
Arrow schema now includes line_number column with explicit line numbers
Response only contains rows for lines with non-zero cumulative or flat values

Replaces fixed-size arrays with a map for per-line metrics in sourceReportBuilder, allowing for sparse line data and adding explicit line_number to the Arrow schema.
@yomete yomete requested review from a team as code owners January 15, 2026 11:36
@alwaysmeticulous
Copy link
Copy Markdown

alwaysmeticulous Bot commented Jan 15, 2026

✅ Meticulous spotted 0 visual differences across 328 screens tested: view results.

Meticulous evaluated ~4 hours of user flows against your PR.

Expected differences? Click here. Last updated for commit 910e0cf. This comment will update as new commits are pushed.

Comment thread pkg/query/sources.go
return array.NewRecordBatch(
arrow.NewSchema(
[]arrow.Field{
{Name: "line_number", Type: arrow.PrimitiveTypes.Int64},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Add the line number as a column in the Arrow schema

Comment thread pkg/query/columnquery.go
if err != nil {
if errors.Is(err, ErrSourceNotFound) || errors.Is(err, ErrNoSourceForBuildID) {
return nil, status.Error(codes.NotFound, err.Error())
source = ""
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Don't return an error if the source code is not present

const flat = table.getChild('flat');
return [cumulative, flat];

const metrics = new Map<number, {cumulative: bigint; flat: bigint}>();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We return the record in sorted order, we can just perform a binary search for each row, no? That way, we can also render only the viewport.

I would prefer it if we got into the habit of not copying data returned by the backend (although probably less problematic in this case).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that makes sense. I can change this

@yomete yomete merged commit bf9cc51 into main Jan 15, 2026
38 checks passed
@yomete yomete deleted the source-report branch February 13, 2026 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants