Skip to content

Improve API baseline diff output#38270

Merged
AndriySvyryd merged 1 commit into
mainfrom
ApiBoss
May 17, 2026
Merged

Improve API baseline diff output#38270
AndriySvyryd merged 1 commit into
mainfrom
ApiBoss

Conversation

@AndriySvyryd
Copy link
Copy Markdown
Member

Improvements to ApiChief's diff generation and the workflow that posts API review comments on merged PRs:

Workflow base SHA: pull_request_target.base.sha is the PR's original base, so any commits pushed to the target branch while the PR was open were being included in the diff. We now use the merge commit's first parent, which is the target tip immediately before the merge — the resulting diff reflects only what the PR actually introduced.
Stage tags in diff: Types and members with a non-Stable stage now render with an inline [Experimental] / [Obsolete] tag, even when the stage itself didn't change. Previously the stage was only shown when it transitioned.
Header-only type changes: Adding/removing an interface (e.g. on RelationalMapToJsonConvention) used to produce a remove-the-whole-type-and-add-it-back diff because type identity was the full declaration string. Type matching is now based on a stable Identity (kind + name + generic arity), and the previous declaration is carried through as a single - old / + new header pair.
Word-wrapping at 160 chars: Long lines (mostly type headers with many interfaces, or methods with many parameters) are wrapped at comma/space boundaries with a 4-space continuation indent. The diff prefix (+/-/ ) is preserved on continuation lines so coloring stays correct.

Workflow base SHA: use the merge commit's first parent, which is the target tip immediately before the merge — the resulting diff reflects only what the PR actually introduced.
Stage tags in diff: Types and members with a non-Stable stage now render with an inline [Experimental] / [Obsolete] tag, even when the stage itself didn't change. Previously the stage was only shown when it transitioned.
Header-only type changes: Type matching is now based on a stable Identity (kind + name + generic arity), and the previous declaration is carried through as a single - old / + new header pair.
Word-wrapping at 160 chars: Long lines are wrapped at comma/space boundaries with a 4-space continuation indent.
Copilot AI review requested due to automatic review settings May 13, 2026 17:15
@AndriySvyryd AndriySvyryd requested a review from a team as a code owner May 13, 2026 17:15
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves ApiChief's diff output quality and fixes the API review workflow's diff scope so posted comments reflect only what each PR actually introduced.

Changes:

  • Use the merge commit's first parent as the diff base (instead of pull_request_target.base.sha) so commits landed on the target branch while the PR was open aren't included.
  • Match types by a stable Identity (kind + name + generic arity, stripping base/interface list and constraints) so header-only changes (e.g., adding an interface) render as a single -old/+new header pair rather than full type removal+re-add.
  • Render non-Stable stage tags inline ([Experimental]/[Obsolete]) on every type/member and wrap long diff lines at 160 chars on comma/space boundaries with a 4-space continuation indent, preserving the +/-/ prefix.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
.github/workflows/api-review-baselines.yml Resolves base SHA to merge commit's first parent for merged PRs to scope the diff to just this PR.
eng/Tools/ApiChief/Model/ApiType.cs Adds Identity (header-stripped) used for equality/hashing and PreviousType to carry the old header for in-memory delta.
eng/Tools/ApiChief/Model/ApiModel.cs Detects header-only type changes and stores baseline header in PreviousType so they appear as modifications.
eng/Tools/ApiChief/Commands/EmitDelta.cs Always emits stage tag, renders header-only changes as -old/+new, and wraps long diff lines at 160 chars preserving the diff prefix.

@roji roji assigned AndriySvyryd and unassigned roji May 17, 2026
@AndriySvyryd AndriySvyryd merged commit e026f3b into main May 17, 2026
17 of 18 checks passed
@AndriySvyryd AndriySvyryd deleted the ApiBoss branch May 17, 2026 19:09
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.

3 participants