From 98a0e3d7fa4061f1b06f20dde01bb9590dcab399 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 18 Jul 2026 11:31:18 +0000 Subject: [PATCH] fix(ci): keep the gallery denormalize diff reviewable and self-healing The nightly denormalization job edits YAML nodes instead of round-tripping structs so its PR stays small enough for a human to review, but the write path undid that: yaml.Marshal re-encoded the node tree at yaml.v3's default 4-space indent and dropped the leading document marker, reflowing roughly 6000 lines around the handful of real changes. Encode through yaml.NewEncoder at the index's authored 2-space indent and restore the header. A write that changes three fields now changes three lines. Stale inferred_min_vram values were also never cleared. Both skip paths (an authored min_vram is present, or the candidate is the last resort) returned before touching the field, so a candidate that gained a floor or became the last resort after a reorder kept an inferred value that EffectiveMinVRAM reported as a real constraint, failing the meta lint with no way for the job to self-heal. Clear the field before both skips. The workflow discarded a whole night's work on any single failure: the program exits 1 when a candidate cannot be estimated, which aborted the job before the PR step, so one unreachable candidate blocked every other refresh indefinitely. Capture the status, open the PR with what was computed, mark the PR body as partial, and fail the run afterwards so the problem still surfaces. Also preserve the index's existing file mode instead of forcing 0644, and drop the redundant //go:build ignore tag, since Go already skips dot directories and the sibling modelslist.go carries no tag. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Ettore Di Giacinto --- .github/ci/gallery_denormalize.go | 68 +++++++++++++++++----- .github/workflows/gallery-denormalize.yaml | 28 ++++++++- 2 files changed, 80 insertions(+), 16 deletions(-) diff --git a/.github/ci/gallery_denormalize.go b/.github/ci/gallery_denormalize.go index 12521a4c1..a650a6458 100644 --- a/.github/ci/gallery_denormalize.go +++ b/.github/ci/gallery_denormalize.go @@ -1,5 +1,3 @@ -//go:build ignore - // Command gallery_denormalize fills the read-only denormalized fields on meta // model entries: backend, quantization, and inferred_min_vram. // @@ -9,6 +7,7 @@ package main import ( + "bytes" "context" "fmt" "os" @@ -80,7 +79,12 @@ func main() { // Authored values win, and the final unconstrained candidate is // deliberately floorless, so neither gets an inferred value. + // Clearing first matters: a candidate that gained an authored + // min_vram, or that became the last resort after a reorder, would + // otherwise keep a stale inferred floor that makes + // EffectiveMinVRAM report a constraint the entry no longer has. if c.MinVRAM != "" || j == len(entries[i].Candidates)-1 { + c.InferredMinVRAM = "" continue } @@ -125,13 +129,14 @@ func main() { // writeCandidates writes back only the three derived keys on each candidate, // leaving every other byte of the index alone. // -// Marshalling the whole document back out would reflow all 1200+ entries -// (quoting, indentation, key order, line wrapping), burying the handful of -// real changes in reformatting noise and making the nightly PR unreviewable. -// Even re-encoding just the candidates subtree would restyle authored fields -// such as min_vram, so the rewrite reaches down to individual mapping keys. -// That also makes the job idempotent: a run that computes the same values -// leaves the file untouched and opens no PR. +// Round-tripping through []GalleryModel would reflow all 1200+ entries +// (quoting, key order, line wrapping), burying the handful of real changes in +// reformatting noise and making the nightly PR unreviewable. Even re-encoding +// just the candidates subtree would restyle authored fields such as min_vram, +// so the rewrite reaches down to individual mapping keys and the node tree is +// re-encoded at the index's authored indent. That also makes the job +// idempotent: a run that computes the same values leaves the file untouched +// and opens no PR. func writeCandidates(path string, data []byte, entries []gallery.GalleryModel) error { var doc yaml.Node if err := yaml.Unmarshal(data, &doc); err != nil { @@ -180,17 +185,50 @@ func writeCandidates(path string, data []byte, entries []gallery.GalleryModel) e } } - // Re-encoding an untouched document still normalizes indentation, so skip - // the write entirely when there was nothing to rewrite. + // Nothing to rewrite means nothing to encode: leave the file, and its + // mtime, alone. if !changed { return nil } - out, err := yaml.Marshal(&doc) - if err != nil { - return fmt.Errorf("marshal: %w", err) + // yaml.Marshal encodes at yaml.v3's default 4-space indent, which reflows + // every nested block in the file (~6000 lines against the real index) and + // drowns the handful of rewritten keys. The index is authored at 2, so + // encode at 2 and the diff stays limited to what actually changed. + var buf bytes.Buffer + enc := yaml.NewEncoder(&buf) + enc.SetIndent(2) + if err := enc.Encode(&doc); err != nil { + return fmt.Errorf("encode: %w", err) } - return os.WriteFile(path, out, 0644) + if err := enc.Close(); err != nil { + return fmt.Errorf("flush encoder: %w", err) + } + + // The encoder does not emit a document start marker, so restore the one the + // file was authored with rather than deleting it on every run. + out := buf.Bytes() + if header := documentHeader(data); header != nil && !bytes.HasPrefix(out, header) { + out = append(header, out...) + } + + // Preserve the file's existing mode instead of forcing 0644. + info, err := os.Stat(path) + if err != nil { + return fmt.Errorf("stat: %w", err) + } + return os.WriteFile(path, out, info.Mode().Perm()) +} + +// documentHeader returns the leading document start marker line, or nil when the +// file was authored without one. +func documentHeader(data []byte) []byte { + for _, marker := range [][]byte{[]byte("---\n"), []byte("---\r\n")} { + if bytes.HasPrefix(data, marker) { + return marker + } + } + return nil } // mappingValue returns the value node for key, or nil when absent. diff --git a/.github/workflows/gallery-denormalize.yaml b/.github/workflows/gallery-denormalize.yaml index 14551d3b1..e58a83e1f 100644 --- a/.github/workflows/gallery-denormalize.yaml +++ b/.github/workflows/gallery-denormalize.yaml @@ -12,8 +12,25 @@ jobs: - uses: actions/setup-go@v5 with: go-version-file: go.mod + # The program exits 1 when any candidate could not be estimated, but it + # still writes everything it did compute. Aborting here would let one + # unreachable candidate block the refresh of every other one, night after + # night, so record the status and let the PR open with the partial result. - name: Denormalize meta candidates - run: go run ./.github/ci/gallery_denormalize.go gallery/index.yaml + id: denormalize + run: | + set +e + go run ./.github/ci/gallery_denormalize.go gallery/index.yaml + status=$? + set -e + echo "status=$status" >> "$GITHUB_OUTPUT" + if [ "$status" -ne 0 ]; then + echo "partial=yes" >> "$GITHUB_OUTPUT" + echo "note=**This run was partial.** At least one candidate could not be estimated, so some fields may be missing or stale. See the workflow log." >> "$GITHUB_OUTPUT" + else + echo "partial=no" >> "$GITHUB_OUTPUT" + echo "note=All candidates were processed successfully." >> "$GITHUB_OUTPUT" + fi - name: Create Pull Request uses: peter-evans/create-pull-request@v8 with: @@ -27,4 +44,13 @@ jobs: `inferred_min_vram` fields on meta model entries. Authored `min_vram` values are never modified. + + ${{ steps.denormalize.outputs.note }} signoff: true + # Surface the failure only after the PR exists, so the problem stays + # visible without discarding the work that did succeed. + - name: Report estimation failures + if: steps.denormalize.outputs.partial == 'yes' + run: | + echo "gallery_denormalize exited ${{ steps.denormalize.outputs.status }}: one or more candidates need a hand-authored min_vram." + exit 1