fix: match dashboard hasDiff and exclude deleted resources from fixes#6
Merged
liamcervante merged 2 commits intomainfrom May 1, 2026
Merged
Conversation
Switches the cost section visibility (hasDiff) and per-project diff check to key off DiffBreakdown.Resources rather than per-resource Action, matching the dashboard's projectHasDiff. ProjectResult.Resources is no longer read anywhere in the comment package and has been removed. Also filters fixed-issue counts to exclude resources that no longer appear in any current policy result (passing or failing) — claiming a "fix" for a resource that has disappeared, or whose policy is no longer applicable, is misleading.
- formatCost: negative values render as `-$X` (sign before currency symbol) and integer parts get thousand-separators (e.g. `-$1,616`). - formatCarbonWithExample: take a pluralize flag; the run-level summary uses it so we say `avoids`/`emits` instead of `avoid`/`emit`. - formatNumber: rewrite to use the dashboard's "decimal places to surface the first non-zero digit" rule. Whole numbers render with no decimals (`16`), fractional values keep needed precision (`335.7`), and thousand-separators are applied. - processCostChangesAndBudgets: sort budget-tag-note keys before joining to fix non-deterministic ordering. - Template: blank lines around the governance sentence so HTML blocks that follow are recognised as block-level by GitHub's markdown parser.
owenrumney
approved these changes
Apr 30, 2026
liamcervante
added a commit
to infracost/actions
that referenced
this pull request
May 1, 2026
#243) * fix: match dashboard's diff calculation and remove ProjectResult.Resources - diffCostBreakdown rewritten to recursively compare cost components and sub-resources, matching the runner's CalculateDiff. A resource lands in the diff when costs, components, or sub-resources differ — not just when total monthly cost differs. Adds and removes are always included. - Drop pr.Resources = head.Resources: the field was removed from comment.ProjectResult upstream because nothing in the comment package reads it any more (hasDiff now keys off DiffBreakdown, hasUnsupported off Summary). - Bump infracost/vcs to pull in the matching comment-formatting fixes (thousand separators, '-$X' negative formatting, carbon verb conjugation, distance precision). Depends on infracost/vcs#6 — once that's tagged and released, bump go.mod to the released version. * vcs latest
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
2 participants
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
Two related fixes to the PR comment so it lines up with what the dashboard renders.
Cost section visibility (hasDiff)
processDisplayCostspreviously decided whether to render the cost section by scanningproject.ResourcesforAction ∈ {CREATE, MODIFY, DELETE}. The scanner output doesn't currently stampAction, so the cost section was being suppressed even when there were obvious changes (e.g. removed RDS instances).Switched the check to
len(project.DiffBreakdown.Resources) > 0, matching the dashboard'sprojectHasDiffindashboard/api/src/services/templates/helpers.ts.hasUnsupportednow reads fromdata.Summary.TotalUnsupportedResources(already aggregated upstream) instead of iteratingproject.Resources.With those two consumers gone,
ProjectResult.Resourceshad no remaining readers in the comment package — removed the field.Fixed-issue count
processFixedIssueswas counting any previously-failing resource that no longer appears in the current policy's failing list as "fixed". On PRs that just delete resources (e.g.glenngillen/demo-missing-tags#2) this inflated the count: removing 12 RDS instances showed up as "12 issues fixed" across various RDS policies, even though nothing was fixed — the resources were just gone.Added a deleted-resources set (built from policy results — a previously-failing resource is "deleted" if it doesn't appear in any current policy result, passing or failing). Both
finopsFixedIssueCountsandtaggingFixedIssueCountsskip resources in that set when counting fixes. Same approach the dashboard takes viaallCurrentResourcesinpolicies.ts, except keyed off policy results instead of a canonical resource list — slightly less precise on the rare "policy applicability changed" edge case, but correctly tightens the common "resource was deleted" case.A follow-up in
actionswill remove the now-orphanedpr.Resources = head.Resourcesassignment in the scanner caller and finish matching the dashboard's diff calculation. Once that lands the cost section becomes visible end-to-end.