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