feat(code-quality): opt-in black --check format job (diff-scoped)

[LukasWodka]

What

Adds an opt-in format job (black --check) to the shared code-quality.yml. Nothing changes for any repo until a caller sets format: true — no caller does in this PR.

Why (backend#1303, the epic's last item)

Black is CI-enforced in backend only (its own lint.yml, pinned from requirements-dev.txt). Everywhere else, the pre-commit hooks from #1307 are the only formatting signal, and those are opt-in per developer.

The design decision: diff-scoped, not repo-wide

Measured today (black 26.3.1, zero parse errors, so these are real verdicts):

repo	dirty files
backend	0 (its gate works)
tracebloc-engine	157
tracebloc-py-package	168
data-ingestors	78
averaging-service	71
client-runtime	15

So there is a genuine ~470-file backlog outside backend. A repo-wide black . sweep right now would collide with the 33 open Code-review PRs and the 23 items sitting in Ready for prod, and would damage git blame across five repos.

Instead the job is diff-scoped like ruff: only the .py files a PR changes must be black-clean (all files in all-files mode). A repo can adopt the gate at zero churn and the tree converges as files are touched.

Inputs

• format (default false) — opt in.
• black-version (default "23.1.0") — pin per caller. black's stable style changes between releases; a mismatched version reports the whole repo as unformatted.
• format-soft-fail (default false) — adopt advisory-first while ruff/gitleaks stay blocking.

Correctness notes

• No --isolated: black reads the repo's own [tool.black] (line-length/target-version/exclude), so CI matches what contributors run locally.
• RC 123 fails closed — an unparsable file is an error, not a formatting verdict, so it is never swallowed by soft-fail.
• NUL-delimited file list + --no-run-if-empty — paths with spaces survive, and an empty changed-file list never silently checks the whole repo.
• Same three-dot diff and fail-open-to-all-files contract as the ruff job.

Test plan

• actionlint clean.
• Logic validated locally against a real clone: clean-file path exits 0; a badly formatted file exits 1 and is counted; a filename containing a space is handled; an empty list is a no-op; an unparsable file returns 123.

Part of tracebloc/backend#1303

🤖 Generated with Claude Code

---

Note

Low Risk
Behavior is gated behind format: false by default; changes only affect repos that explicitly opt in to the new CI job.

Overview
Extends the reusable code-quality.yml with an opt-in format job (black --check) that is off by default until a caller sets format: true.

New inputs: format, format-soft-fail (format-only advisory mode), and black-version (default 23.1.0 to match the fleet). Scope mirrors ruff: changed .py files on PRs, or the whole tree when all-files is set, with the same three-dot diff and fail-open-to-all-files behavior.

The job installs black via pipx, respects the repo’s [tool.black] (no --isolated), and writes a step summary. Pass/fail is driven by black’s output, not raw exit codes—so GNU xargs remapping reformat failures to 123 does not break advisory adopters, parse/processing errors are fail-closed and not treated as “needs formatting,” and soft-fail / format-soft-fail only soften real reformat findings.

^{Reviewed by Cursor Bugbot for commit affc564. Bugbot is set up for automated code reviews on this repo. Configure here.}

[LukasWodka]

👋 Heads-up — Code review queue is at 35 / 30

Above the WIP limit. The team convention is to review existing PRs before opening new work.

Open PRs currently in Code review (oldest first):

• averaging-service#256 — chore: adopt the org PR template · author: @LukasWodka · no reviewer assigned
• averaging-service#257 — feat(blob_store): read model code + dataset + FedOpt state via blob_store, retire exp_path (#1162) · author: @saadqbal · no reviewer assigned
• backend#1146 — docs(rfc): platform cost & autoscaling architecture · author: @saadqbal · reviewer: @LukasWodka, @saqlainsyed007, @aptracebloc
• backend#1201 — docs(rfc): Data spaces & federated dataset composition (draft) · author: @LukasWodka · reviewer: @saadqbal
• backend#1217 — docs(rfc): Dataset integrity & score reproducibility (draft) · author: @LukasWodka · reviewer: @saadqbal
• backend#1261 — RFC-BACKEND-0008: board & process reset (decision RFC — for markup) · author: @LukasWodka · reviewer: @saadqbal
• backend#1284 — docs(deploy): backend deploy runbook skeleton (#1263) · author: @LukasWodka · no reviewer assigned
• backend#1332 — feat(check-model): allow KNN/GaussianProcess on a single-edge owned dataset (#923 follow-up) · author: @divyasinghds · no reviewer assigned
• backend#1380 — chore: adopt the org PR template · author: @LukasWodka · no reviewer assigned
• claude-skills#14 — feat(skill): add tracebloc-frontend-app skill · author: @waqaskhanroghani · reviewer: @saadqbal

Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.)

[LukasWodka]

bugbot run

[aptracebloc]

Confirmed the Bugbot "xargs remaps soft-fail exit code" finding — it's a real High, not a false positive. Trace:

Diff mode (L381-383) runs black --check under xargs -0. When black finds files that would reformat it exits 1, but xargs remaps any child exit in 1–125 to its own exit 123 (per man xargs). So RC=123, not 1.

Exit handling (L401-406) then hits the fail-closed branch first:

# RC 123 = internal error (unparsable file), not a formatting verdict.if [ "$RC"!="0" ] && [ "$RC"!="1" ];then# 123 → both trueecho"::error::black exited $RC ... - failing closed."exit"$RC"# exits before the soft-fail checkfiif [ "$RC"!="0" ] && [ "$SOFT_FAIL"="true" ];thenexit 0;fi# never reached in diff mode

So on the diff-scoped path — the job's primary mode — an ordinary "would reformat" always hard-fails, even with soft-fail/format-soft-fail set, and a genuine internal error (unparsable file) becomes indistinguishable from a normal formatting verdict (both surface as 123). The comment on L401 (RC 123 = internal error) is the buggy assumption under xargs. The all-mode path (black --check ., no xargs) is unaffected — black's 1 is captured directly there and soft-fail works.

Since /tmp/black.out already holds the verdict, the smallest fix is to disambiguate by output rather than trusting xargs's exit code (COUNT is already computed just above):

if [ "$RC"!="0" ] && [ "$RC"!="1" ];then# Diff mode runs black under xargs, which remaps black's exit 1 -> 123.# It's a real internal error only if there are no "would reformat" lines.if [ "$COUNT"-gt 0 ];then
RC=1
elseecho"::error::black exited $RC (not a formatting failure) - failing closed."
cat /tmp/black.out
exit"$RC"fifi

That restores advisory behavior on the diff path while still failing closed on genuine internal errors. Given the whole caller rollout is advisory (soft-fail), worth fixing before repos opt into the format job in diff mode — otherwise the first unformatted file hard-blocks them.

[LukasWodka]

@aptracebloc confirmed and fixed — thank you, that was a real High and your trace was exact.

Verified the platform difference empirically, which is also why my local validation gave a false green: printf 'x\0' | xargs -0 sh -c 'exit 1' returns 1 on BSD/macOS xargs but 123 on GNU xargs (ubuntu-latest). So the diff path — the job's primary mode — would have hard-failed every advisory adopter on the first unformatted file, exactly as you described, while passing on my machine.

Fix (b2ef22c): decide from black's OUTPUT, not its exit code. I took your approach but reordered it to close one residual hole: your version sets RC=1 when COUNT>0, which means a run containing both a reformat finding and a genuine internal error would be downgraded to a formatting verdict and then swallowed by soft-fail. So it now checks errors first:

ERRS=$(grep -cE '^error:|would fail to reformat' /tmp/black.out || true)if [ "${ERRS:-0}"-gt 0 ];then ... fail closed, always ...
elif [ "${COUNT:-0}"-gt 0 ];then ... soft-fail applies here ...
elif [ "$RC"!="0" ];then ... nonzero with no findings -> fail closed ...

Exit codes are now only a tiebreaker for "nonzero but reported nothing".

Simulated all 8 combinations (dirty / unparsable / clean × advisory / blocking × diff / all-files) with rc forced to 123 to mimic GNU xargs: advisory now passes on a dirty diff, blocking fails, an unparsable file fails closed regardless of soft-fail, and error+dirty in one run fails closed. Clean stays clean.

Side note: the errors-vs-verdicts distinction this fix rests on is the same one that produced a bogus "0 files need formatting" in the #1303 sizing (black 23.1.0 can't run on Python 3.12+ — ast.Str removed — so it errored on every file while the reformat count stayed 0). That lesson is now encoded in the job with a comment, so it can't silently recur.

[LukasWodka]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit affc564. Configure here.}