observability-diff: scope PR comment to files the PR actually changes#4711
Merged
observability-diff: scope PR comment to files the PR actually changes#4711
Conversation
Contributor
Observability diff (vs staging)No dashboard / folder changes detected against the staging Grafana. (Run: https://github.com/cardstack/boxel/actions/runs/25511601796) |
Contributor
There was a problem hiding this comment.
Pull request overview
Scopes the “observability diff vs staging” PR comment to only the Grafana dashboard/folder manifests that are changed by the current PR, reducing noise from unrelated drift in staging.
Changes:
- Add
diff.sh --only-changed-since=<ref>to filter diffs to only files changed underpackages/observability/grafanactl/resources/, with an early exit when no relevant changes exist. - Update the
observability-diffGitHub Actions workflow to use the new flag and to fetch full git history.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/observability/scripts/diff.sh | Adds --only-changed-since filtering logic and applies it during normalization/canonicalization steps. |
| .github/workflows/observability-diff.yml | Uses fetch-depth: 0 and passes --only-changed-since=origin/<base> to scope the generated PR comment diff. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lukemelia
added a commit
that referenced
this pull request
May 7, 2026
Four follow-ups from Copilot's review:
* Document `--only-changed-since` in diff.sh's header Usage block,
including the empty-stdout semantics and the deletion-exclusion
rationale.
* Validate the ref via `git rev-parse --verify` before using it.
CI usually passes `origin/<base-ref>` and a missing/invalid ref
used to surface as a noisy `git diff` failure under set -e;
now diff.sh emits an actionable error pointing at the workflow's
fetch step.
* `git diff --name-only --diff-filter=ACMRT` excludes Deletions
from the filter set. `grafanactl push` is upsert-only and never
deletes, so a deletion-only PR is a no-op for live state — should
short-circuit to empty stdout, not trigger a `grafanactl pull`.
* Workflow: explicit `git fetch --no-tags --depth=1 origin "${PR_BASE_REF}"`
before running diff.sh. Belt-and-suspenders even with
`fetch-depth: 0` — actions/checkout's behavior with PR base refs
isn't 100% guaranteed across runner image versions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lukemelia
force-pushed
the
fix/observability-diff-only-pr-changed-files
branch
from
a1c9df2 to
529a1fd
Compare
1 task
The PR comment workflow (observability-diff.yml) runs diff.sh in "diff everything" mode, which surfaces total drift between main + this PR and live staging Grafana. That includes drift caused by earlier merges that haven't reached staging yet, manual UI edits in staging, etc. — none of which belong in the PR comment because the PR didn't change those files. Adds `diff.sh --only-changed-since=<ref>` to scope the diff to files that changed in `git diff --name-only <ref>...HEAD` under `packages/observability/grafanactl/resources/`. The workflow passes the PR's base branch (`origin/<base.ref>`) so the comment shows what THIS PR would change on apply, not accumulated drift. If no observability dashboards/folders changed in the PR, diff.sh exits 0 with empty stdout — the comment step renders the existing "No dashboard / folder changes detected" body. Avoids both the expensive grafanactl pull AND the noisy comment. Implementation notes: * Filter list is a newline-separated string (not an associative array) for bash 3.2 compatibility — same constraint the rest of this script already honors for `find -print0` patterns. * `actions/checkout` gains `fetch-depth: 0` so `git diff origin/<base>...HEAD` has both refs available; default fetch-depth: 1 gives only HEAD. * The filter is applied in both `normalize()` (which copies pulled files into the per-folder layout) and `normalize_json_content()` (which canonicalizes both sides). Files outside the filter are silently skipped at both stages so they never reach the final `git diff --no-index`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lukemelia
added a commit
that referenced
this pull request
May 7, 2026
Four follow-ups from Copilot's review:
* Document `--only-changed-since` in diff.sh's header Usage block,
including the empty-stdout semantics and the deletion-exclusion
rationale.
* Validate the ref via `git rev-parse --verify` before using it.
CI usually passes `origin/<base-ref>` and a missing/invalid ref
used to surface as a noisy `git diff` failure under set -e;
now diff.sh emits an actionable error pointing at the workflow's
fetch step.
* `git diff --name-only --diff-filter=ACMRT` excludes Deletions
from the filter set. `grafanactl push` is upsert-only and never
deletes, so a deletion-only PR is a no-op for live state — should
short-circuit to empty stdout, not trigger a `grafanactl pull`.
* Workflow: explicit `git fetch --no-tags --depth=1 origin "${PR_BASE_REF}"`
before running diff.sh. Belt-and-suspenders even with
`fetch-depth: 0` — actions/checkout's behavior with PR base refs
isn't 100% guaranteed across runner image versions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lukemelia
force-pushed
the
fix/observability-diff-only-pr-changed-files
branch
from
529a1fd to
237b46a
Compare
Four follow-ups from Copilot's review:
* Document `--only-changed-since` in diff.sh's header Usage block,
including the empty-stdout semantics and the deletion-exclusion
rationale.
* Validate the ref via `git rev-parse --verify` before using it.
CI usually passes `origin/<base-ref>` and a missing/invalid ref
used to surface as a noisy `git diff` failure under set -e;
now diff.sh emits an actionable error pointing at the workflow's
fetch step.
* `git diff --name-only --diff-filter=ACMRT` excludes Deletions
from the filter set. `grafanactl push` is upsert-only and never
deletes, so a deletion-only PR is a no-op for live state — should
short-circuit to empty stdout, not trigger a `grafanactl pull`.
* Workflow: explicit `git fetch --no-tags --depth=1 origin "${PR_BASE_REF}"`
before running diff.sh. Belt-and-suspenders even with
`fetch-depth: 0` — actions/checkout's behavior with PR base refs
isn't 100% guaranteed across runner image versions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lukemelia
force-pushed
the
fix/observability-diff-only-pr-changed-files
branch
from
237b46a to
064a2be
Compare
Contributor
habdelra
approved these changes
May 7, 2026
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.
Summary
The observability-diff PR comment workflow currently shows total drift between main + this PR and live staging Grafana. That includes drift from earlier merges that haven't reached staging, manual UI edits in staging, AMG-era residue, etc. — most of which has nothing to do with the PR being reviewed and adds noise.
This PR scopes the diff to only the dashboard/folder files that changed in the PR, so the comment reflects what would happen on apply of this PR, not accumulated drift.
Mechanism
diff.sh --only-changed-since=<ref>flag. When set, runsgit diff --name-only <ref>...HEAD -- packages/observability/grafanactl/resources/and only includes those files in the canonicalize + diff pipeline.diff.shexits 0 with empty stdout (skipping the expensivegrafanactl pull). The workflow's comment step then renders the existing "No dashboard / folder changes detected" body.origin/${{ github.event.pull_request.base.ref }}so the comparison is against the PR's actual target branch.actions/checkoutgainsfetch-depth: 0soorigin/<base>...HEADhas both refs available (defaultfetch-depth: 1only fetches HEAD).Implementation notes
[[ "$line" == "$key" ]]scan, notdeclare -A, for bash 3.2 compatibility — same constraint the rest of the script already honors (see thefind -print0pattern comments throughout).normalize()(per-folder layout) andnormalize_json_content()(jq canonicalization) so unrelated files never reach the finalgit diff --no-index.Effect on the existing stack
For the open dashboard PRs (#4694–#4699):
grafanactl/resources/boxel-logs.json,synapse.jsonindexing.json,job-queue.json, deletion ofboxel-jobs.jsonworker-status.jsonRight now every one of those PRs gets the same multi-thousand-line diff dominated by drift unrelated to the PR. After this lands, each comment will be tightly scoped.
Test plan
./scripts/diff.sh --env staging --only-changed-since=origin/main(requires AWS access for SSM token) — verify scope is just files changed since main../scripts/diff.sh --env staging --only-changed-since=<ref-with-no-observability-changes>— verify exits 0 with empty stdout, no grafanactl pull happened.🤖 Generated with Claude Code