Skip to content

Commit 347aa1d

Browse files
committed
fix: report tech-debt changes by data-row content, not line count
Previously `prSummaryReport` decided whether to render a report by counting lines in the rendered text: if [ "$(wc -l <<<"${rep}")" -le 5 ]; then "no changes" That worked in the original layout (mathlib4#18971) where `report` emitted 5 boilerplate lines (header + separator + blank + 2 commit URLs). When the strong/weak split (#28) moved the commit URLs into a separate `reportFooter`, only 2 boilerplate lines remained in `rep`, but the `-le 5` threshold was kept. Any PR changing three or fewer counters of a given level was silently reported as "no changes" — e.g. leanprover-community/mathlib4#39932 removed one `set_option backward.isDefEq.respectTransparency false in` without the bot noticing. Instead of patching the threshold (which would stay coupled to whatever incidental formatting `report` happens to emit), filter directly to the non-zero data rows in a named `filterChangedRows`, branch on `[ -z "${changedRows}" ]`, and reconstruct the table header in the "has changes" branch. The Change column is used as the test (rather than the Current column) because a counter that disappeared entirely in the new commit renders as `||-1|name|` — empty Current — and we still want to report it. 🤖 Prepared with Claude Code
1 parent aafe3df commit 347aa1d

1 file changed

Lines changed: 33 additions & 16 deletions

File tree

scripts/reporting/technical-debt-metrics.sh

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -187,27 +187,44 @@ if [[ "${level}" == "weak" ]]; then
187187
fi
188188
}
189189

190+
# Filter `report`'s output down to just the data rows whose Change column is
191+
# a non-zero integer, dropping the table headers, any blank/footer lines, and
192+
# everything inside fenced ``` blocks (the Deprecated-files spoiler). A "data
193+
# row" from `computeDiff` is `|N|diff|name|`; `N` may be empty if a counter
194+
# disappeared in the new commit, so we test the Change column rather than the
195+
# Current column.
196+
filterChangedRows () {
197+
awk -F'|' '
198+
BEGIN { inFence = 0 }
199+
/^```/ { inFence = !inFence; next }
200+
(!inFence) && ($3+0 == $3) && ($3+0 != 0) { print }
201+
'
202+
}
203+
190204
prSummaryReport () {
191205
level="${1}"
192-
rep="${2}"
206+
changedRows="${2}"
193207

194-
if [ "$(wc -l <<<"${rep}")" -le 5 ]
208+
if [ -z "${changedRows}" ]
195209
then
196210
printf '<details><summary>No changes to %s technical debt.</summary>\n' "${level}"
197211
return 1
198-
else
199-
printf '%s\n' "${rep}" | # outputs lines containing `|Current number|Change|Type|`, so
200-
# `$2` refers to `Current number` and `$3` to `Change`.
201-
awk -F '|' -v rep="${rep}" '
202-
BEGIN{total=0; weight=0; absWeight=0}
203-
{absWeight+=$3+0}
204-
(($3+0 == $3) && (!($2+0 == 0))) {total+=1 / $2; weight+=$3 / $2}
205-
END{
206-
if (total == 0) {average=absWeight} else {average=weight/total}
207-
if(average < 0) {change= "Decrease"; average=-average; weight=-weight} else {change= "Increase"}
208-
printf("<details><summary>%s in '"${level}"' tech debt: (relative, absolute) = (%4.2f, %4.2f)</summary>\n\n%s\n", change, average, weight, rep) }'
209-
return 0
210212
fi
213+
214+
# Reconstruct the rendered table with the header rows that the awk filter dropped.
215+
rep="$(printf '|Current number|Change|Type|\n|-:|:-:|:-|\n%s\n' "${changedRows}")"
216+
217+
printf '%s\n' "${rep}" | # outputs lines containing `|Current number|Change|Type|`, so
218+
# `$2` refers to `Current number` and `$3` to `Change`.
219+
awk -F '|' -v rep="${rep}" '
220+
BEGIN{total=0; weight=0; absWeight=0}
221+
{absWeight+=$3+0}
222+
(($3+0 == $3) && (!($2+0 == 0))) {total+=1 / $2; weight+=$3 / $2}
223+
END{
224+
if (total == 0) {average=absWeight} else {average=weight/total}
225+
if(average < 0) {change= "Decrease"; average=-average; weight=-weight} else {change= "Increase"}
226+
printf("<details><summary>%s in '"${level}"' tech debt: (relative, absolute) = (%4.2f, %4.2f)</summary>\n\n%s\n", change, average, weight, rep) }'
227+
return 0
211228
}
212229

213230
# What to print at the end of a set of reports.
@@ -221,8 +238,8 @@ if [ "${1:-}" == "pr_summary" ]
221238
then
222239
alreadyPrintedFooter="false"
223240

224-
repStrong="$(report strong | awk -F'|' 'BEGIN{backTicks=0} /^```/{backTicks++} ((!/^```/) && (backTicks % 2 == 0) && !($3 == "0")) {print $0}')"
225-
repWeak="$(report weak | awk -F'|' 'BEGIN{backTicks=0} /^```/{backTicks++} ((!/^```/) && (backTicks % 2 == 0) && !($3 == "0")) {print $0}')"
241+
repStrong="$(report strong | filterChangedRows)"
242+
repWeak="$(report weak | filterChangedRows)"
226243
if prSummaryReport strong "${repStrong}"; then
227244
reportFooter
228245
printf '\nThis script lives in the [`mathlib-ci`](https://github.com/leanprover-community/mathlib-ci) repository. To run it locally, from your `mathlib4` directory:\n```\ngit clone https://github.com/leanprover-community/mathlib-ci.git ../mathlib-ci\n../mathlib-ci/scripts/reporting/technical-debt-metrics.sh pr_summary\n```\n%s\n</details>\n' '* The `relative` value is the weighted *sum* of the differences with weight given by the *inverse* of the current value of the statistic.

0 commit comments

Comments
 (0)